Skip to content

FIREFLY-29: inadequate interpretation of the target field value #822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 12, 2019

Conversation

tgoldina
Copy link
Contributor

@tgoldina tgoldina commented Jun 7, 2019

https://jira.ipac.caltech.edu/browse/FIREFLY-29
Test build: https://irsawebdev9.ipac.caltech.edu/firefly-29_tgtresolver/firefly/?__action=layout.showDropDown&visible=true&view=IrsaCatalogDropDown

  • Objects with the names starting with a number are now resolved
  • Names with long-dash are now resolved (they were previously causing "TypeError: Cannot assign to read only property ..." and "Invariant Violation" warnings in React.
  • Show unencoded name for failed name resolutions. (They were previously shown encoded.)

Previously failing, now resolving target names:
61 Ursae Majoris
41 Ara
12 Ophiuchi
39 Leonis
SCR J0630–7643

Known issues:

  • The greek symbols are not converted into English equivalent.
  • Degree symbol is not stripped.

These are the names resolved by http://cds.u-strasbg.fr/cgi-bin/Sesame, but not by Firefly:
Sk -68°137
ξ UMa
β CVn
χ¹ Ori
δ Eri
κ¹ Cet
ζ Puppis

(from https://en.wikipedia.org/wiki/Lists_of_stars)

Firefly replaces unrecognized non-ascii characters with '?', hence giving user a hint how to change the name so that it gets recognized.

@tgoldina tgoldina requested a review from ejoliet June 7, 2019 21:32
@tgoldina tgoldina self-assigned this Jun 7, 2019
@ejoliet ejoliet requested a review from cgelino June 10, 2019 19:43
@Olga9999 Olga9999 self-requested a review June 10, 2019 22:49
@cgelino
Copy link

cgelino commented Jun 11, 2019

I tested with OS X 10.11.6, Firefox 67.0.1, Safari 11.1.2, and Chrome 71.0.3578.98. All tests returned results as expected:
-- objects with names starting with a number were resolved properly
-- object names with Greek characters or special symbols were not resolved, but displayed a '?' in place of the special symbol in the informational message to the user.

@Olga9999
Copy link

REPORT from olga9999
#822 FireFly-29

https://jira.ipac.caltech.edu/browse/FIREFLY-29

https://irsawebdev9.ipac.caltech.edu/firefly-29_tgtresolver/firefly/?__action=layout.showDropDown&visible=true&view=IrsaCatalogDropDown

  1. MacOS 10.14.5
    Browsers:
    FireFox 67.0(64 bit) AND Safari v. 12.1.1

61 Ursae Majoris
41 Ara
12 Ophiuchi
39 Leonis
SCR J0630-7643

  • are working fine.

I have tested other names started from the number:
3c279
2MASS J12283266-4315159

  • also work.

Sk -68°137
and names with greek letters

  • are now working as expected.
    Not really working, but it gives the "?" in proper places after
    "Could not resolve:"
  1. Mac OS 10.10.5
    Browsers:
  • FireFox 66.0.5
    all works as described above.
  • Safari 10.1.2
    will not open testing site, probably too old.

Thanks,
Olga

@ejoliet
Copy link
Contributor

ejoliet commented Jun 11, 2019

@cgelino @Olga9999 thanks for testing. Seems everything works and the bug is fixed, could you approve the PR as a formal step? thanks!

Copy link
Contributor

@ejoliet ejoliet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is working fine now and acknowledged that greek target name won't work. Good to be merged.
Thank you!

Copy link

@cgelino cgelino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. [I put my comment in the wrong box!]

@Olga9999 Olga9999 closed this Jun 11, 2019
@ejoliet ejoliet reopened this Jun 11, 2019
Copy link

@Olga9999 Olga9999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works fine now

@tgoldina tgoldina merged commit b2d3570 into dev Jun 12, 2019
@tgoldina tgoldina deleted the firefly-29_tgtresolver branch June 12, 2019 22:19
@robyww robyww added bug UI Client side UI changes not related to any of the visualizers labels Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug UI Client side UI changes not related to any of the visualizers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants