Skip to content
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

source catalog and line profile use source for images, handle missing ra, dec, add clientAlertExceptions to analysis endpoint #42

Merged
merged 4 commits into from
Dec 4, 2024
Merged

source catalog and line profile use source for images, handle missing ra, dec, add clientAlertExceptions to analysis endpoint #42

merged 4 commits into from
Dec 4, 2024

Conversation

LTDakin
Copy link
Contributor

@LTDakin LTDakin commented Dec 2, 2024

Changes

  • Source catalog and Line profile now account for the images source when fetching the fits file
  • if ra and dec is missing now doesn't error but returns an empty data set, and message to front end that the image doesn't support that action
  • comments

@LTDakin
source catalog and line profile use source for images, handle missing…
ddc1e39 
… ra, dec, add clientAlertExceptions to analysis endpoint
jnation3406
jnation3406 approved these changes Dec 3, 2024
View reviewed changes
Copy link
Contributor

@jnation3406 jnation3406 left a comment

Choose a reason for hiding this comment

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

Looks fine, just wondering if we should not return any ra/dec instead of zeros if it doesn't exist.

datalab/datalab_session/analysis/source_catalog.py Outdated
dec = cat_hdu.data["dec"][:SOURCE_CATALOG_COUNT]
else:
# TODO: implement a fallback way to calculate ra, dec from x, y and WCS
ra = np.zeros(SOURCE_CATALOG_COUNT)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would be cleaner to just not return ra/dec if they don't exist, and have the frontend check if they exist to show a message or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Set them to null and don't include them in the return object, also added a check where they were used to fill out tool tip info in the frontend

@LTDakin LTDakin merged commit dbd093a into main Dec 4, 2024
3 checks passed
@LTDakin LTDakin deleted the fix/output-analysis-tweaks branch December 4, 2024 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnation3406 jnation3406 jnation3406 approved these changes

@mgdaily mgdaily Awaiting requested review from mgdaily

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LTDakin @jnation3406