-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
New module: HiGal catalog + image cutout service #1324
base: main
Are you sure you want to change the base?
Conversation
TODO:
|
what other stuff lives at ssdc? Do we foresee that any of those will need a module in the future? If yes, in that case it may make sense to restructure from the beginning and call it ssdc and have higal under it. |
Good questions. I have no clue. The sub-interface for HiGal appears to be at least somewhat distinct from the general SSDC querier. We need one of them onboard.... |
An interesting deficiency I discovered working on this is that |
what is the status of the other Herschel surveys then? maybe a herschel module then? Though that would be far from any homogeneity. |
unfortunately, it looks like Herschel surveys are hosted randomly. Many are at IPAC, others are not. |
Hello @keflavich! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-09-21 20:55:30 UTC |
Apparently I haven't noticed this comment before. Do you suggest that we handle that here, or upstream? If the latter can you open an issue for astropy? |
I guess upstream? But how often is this actually needed? I'm not really sure how we'd handle it upstream anyway; So... not upstream, then! |
preferring to use |
Can I help with this PR to help bring it to fruition (as suggested by @keflavich earlier)? If I am to finish up this PR I might need to open an other PR to "supplant" this... |
Yes, go ahead and work on this one if you like. I don't remember where I left it off, but maybe all we need to do is get tests passing? You can submit a new PR based on this one. Normally I'd prefer to just give write permissions, but I don't know how to do that. |
Yeah, I would prefer to get write permissions too. That would be easier for me as well. Would this help?: https://help.github.com/en/articles/managing-an-individuals-access-to-an-organization-repository |
Right, I forgot I can just give you write permissions to my fork - we don't (and can't) give out permission to write to |
No problem, I can add new changes to this PR via your fork... Will follow up on this very soon. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1324 +/- ##
==========================================
- Coverage 66.80% 66.79% -0.01%
==========================================
Files 237 237
Lines 18320 18321 +1
==========================================
Hits 12238 12238
- Misses 6082 6083 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor things that should fix the failing docs build
|
||
An example catalog query: | ||
|
||
.. python:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
either remove this (should work without) or add .. code-block:: python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to .. code-block:: python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added blank line after this line
|
||
And an example image query: | ||
|
||
.. python:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to .. code-block:: python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added blank line after this line
Reference/API | ||
============= | ||
|
||
.. automodapi:: astroquery.higal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also link this file in the main index docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added higal/higal.rst
to index.rst
Organizing:should we start it here before it goes in? @keflavich - would you be against an |
No, |
|
All checks have passed, ready for the next stage... |
@keflavich - Is this still something on the table? I also wonder whether it should be part of more ssdc functionality (and namespace) rather than to exists on its own? |
I keep updating this because I use it in some code for publications, but I've never written a test suite. If SSDC provided other services, I'd be happy to put this tool into that namespace, but I don't know anything else it provides. So leave this open, I'll keep updating, and someday when I don't have fires to put out, I'll finish this and we can talk namespaces again. |
I pulled up this PR as I'm at a talk about SSDC as is, and it seems that there are more services and tools they provide, many with a VO. But I haven't gone into the weeds of how much of that we could/want/should support. |
Oh wow, great! If there are other SSDC products, then, I'm super happy to support them! I hope some of them have nicer APIs - VO would be great. reminder to me: https://www.ssdc.asi.it/ is the SSDC homepage. |
Also, the hack I had before isn't needed when you appropriately query for multiple wavelengths
pep8 lies sometimes.
commit has some fixes
This is a new tool to query the HiGal image cutout and catalog service. The catalog can be accessed from Vizier and therefore is not necessary, but the image cutout service is unique.
The service is hosted at https://tools.ssdc.asi.it/HiGALSearch.jsp. I have never been able to find that URL by googling it.
I still need to make some local tests, but remote tests exist.