Skip to content

DM-12113: refactor image services code. #471

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 7, 2017
Merged

DM-12113: refactor image services code. #471

merged 5 commits into from
Oct 7, 2017

Conversation

robyww
Copy link
Contributor

@robyww robyww commented Oct 6, 2017 •
edited
Loading

this pull request is a bunch of random code clean up and little bug fixes.

  • eliminate VisNetwork, IrsaCatalogParams
  • refactor image service
  • fixed related data bug
  • fix flux calculation with file one disk is compressed
  • cleanup of URLDownload
  • removed PushServices the only function moved to JsonUtils
  • removed a lot of unused client side js code
  • fixed a bug when calling the server for the flux value, the wrong value is returned if the value is compressed on disk
  • fixed a bug with related data for masking.

to test:

This pull request is really hard to test since it is clean up all over. The best test is to go to the image panel and try to load all the types of images

robyww added 3 commits October 5, 2017 07:53
@robyww
Moved the services around, cleanup, and refactor
82d7218 
     - eliminate VisNetwork, IrsaCatalogParams
     - refactor image service
     - fixed related data bug
     - fix flux calculation with file one disk is compressed
     - cleanup of URLDownload
@robyww
DM-12113: Merge branch 'dev' into DM-12113-image-svc
978afb2
@robyww
DM-12113: clean up client side code
37e51f1 
  - removed PushServices the only function moved to JsonUtils
  - removed a lot of unused code
@robyww robyww force-pushed the DM-12113-image-svc branch from cd68997 to 37e51f1 Compare October 6, 2017 19:01
@robyww robyww requested a review from cwang2016 October 6, 2017 19:08
@robyww robyww self-assigned this Oct 6, 2017
@robyww robyww mentioned this pull request Oct 7, 2017
Merged
@cwang2016
Copy link
Contributor

Not sure if this finding is related to this ticket:
after loading an image, open 'FITS Donwload Dialog', only download 'FITS' file works, the other two 'PNG file' and 'Region file' don't have files downloaded.

cwang2016
cwang2016 approved these changes Oct 7, 2017
View reviewed changes
Copy link
Contributor

@cwang2016 cwang2016 left a comment

Choose a reason for hiding this comment

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

Loading of each type of images is tested and work well except some fits with multiple extension could not be loaded successfully. I think that is because of great amount of image data.

src/firefly/java/edu/caltech/ipac/firefly/server/visualize/LockingVisNetwork.java Outdated
f= IbeImageGetter.lowlevelGetIbeImage(params);
break;
case WISE:
f= IbeImageGetter.lowlevelGetIbeImage(params);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, if case TWOMASS, TWOMASS & case WISE do the same process? or these two cases may have different process?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lowlevelGetIbeImage figure out with one it has.

@robyww
Copy link
Contributor Author

robyww commented Oct 7, 2017 •
edited
Loading

Not sure if this finding is related to this ticket:
after loading an image, open 'FITS Donwload Dialog', only download 'FITS' file works, the other two 'PNG file' and 'Region file' don't have files downloaded.

I don't think this has anything to do with it, but I will look anyway.

Tested it- seems to work for me.

@robyww robyww merged commit 4e56f28 into dev Oct 7, 2017
@robyww robyww deleted the DM-12113-image-svc branch October 7, 2017 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cwang2016 cwang2016 cwang2016 approved these changes

Assignees

@robyww robyww

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@robyww @cwang2016