astroquery icon indicating copy to clipboard operation
astroquery copied to clipboard

New module: HiGal catalog + image cutout service

Open keflavich opened this issue 6 years ago • 26 comments

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.

keflavich avatar Jan 03 '19 23:01 keflavich

TODO:

  • [ ] documentation
  • [ ] local tests

keflavich avatar Jan 03 '19 23:01 keflavich

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.

bsipocz avatar Jan 03 '19 23:01 bsipocz

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....

keflavich avatar Jan 03 '19 23:01 keflavich

An interesting deficiency I discovered working on this is that astropy.utils.data.get_readable_fileobj does not work for files that require a cookie. That's an oversight we could possibly handle in the distant future.

keflavich avatar Jan 04 '19 00:01 keflavich

what is the status of the other Herschel surveys then? maybe a herschel module then? Though that would be far from any homogeneity.

bsipocz avatar Jan 04 '19 00:01 bsipocz

unfortunately, it looks like Herschel surveys are hosted randomly. Many are at IPAC, others are not.

keflavich avatar Jan 04 '19 00:01 keflavich

Hello @keflavich! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 47:22: E124 closing bracket does not match visual indentation Line 53:25: E124 closing bracket does not match visual indentation Line 88:48: E124 closing bracket does not match visual indentation Line 133:43: E262 inline comment should start with '# ' Line 221:43: E124 closing bracket does not match visual indentation Line 223:13: E265 block comment should start with '# ' Line 225:13: E265 block comment should start with '# ' Line 245:48: E124 closing bracket does not match visual indentation Line 262:22: E124 closing bracket does not match visual indentation

Line 27:15: E124 closing bracket does not match visual indentation Line 28:14: E124 closing bracket does not match visual indentation Line 59:1: E302 expected 2 blank lines, found 1 Line 66:1: E302 expected 2 blank lines, found 1

Comment last updated at 2024-02-20 20:26:13 UTC

pep8speaks avatar Feb 28 '19 00:02 pep8speaks

An interesting deficiency I discovered working on this is that astropy.utils.data.get_readable_fileobj does not work for files that require a cookie. That's an oversight we could possibly handle in the distant future.

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?

bsipocz avatar Feb 28 '19 00:02 bsipocz

I guess upstream? But how often is this actually needed? I'm not really sure how we'd handle it upstream anyway; requests makes it so much easier I might just say we should require it for cookie-related downloads.

So... not upstream, then!

keflavich avatar Feb 28 '19 00:02 keflavich

preferring to userequests sounds actually super sensible.

bsipocz avatar Feb 28 '19 00:02 bsipocz

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...

kakirastern avatar Mar 18 '19 19:03 kakirastern

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.

keflavich avatar Mar 18 '19 20:03 keflavich

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

kakirastern avatar Mar 18 '19 20:03 kakirastern

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 astropy/astroquery. Anyway, you should now have permission to write to this branch.

keflavich avatar Mar 18 '19 20:03 keflavich

No problem, I can add new changes to this PR via your fork... Will follow up on this very soon.

kakirastern avatar Mar 18 '19 20:03 kakirastern

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (1c6b7bf) 66.80% compared to head (a56b91b) 66.79%.

Files Patch % Lines
astroquery/query.py 0.00% 2 Missing :warning:
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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 26 '19 23:03 codecov[bot]

Organizing:should we start it here before it goes in?

@keflavich - would you be against an astroquery.herschel.higal rather than having it in the top namespace?

bsipocz avatar Mar 27 '19 00:03 bsipocz

No, .herschel.higal is fine. It might also end up co-listed under esac.higal and cutout_services.higal eventually?

keflavich avatar Mar 27 '19 00:03 keflavich

esa.higal and cutout_services.higal sounds very good approach to me. The only trick is that we need to make sure users know it's all the same whichever namespace they end up using.

bsipocz avatar Mar 27 '19 00:03 bsipocz

All checks have passed, ready for the next stage...

kakirastern avatar Mar 27 '19 06:03 kakirastern

Should I change the organization to astroquery.herschel.higal next? I can change both the names for the docs and the code...

kakirastern avatar Mar 28 '19 12:03 kakirastern

Just noting that.... we still have to finish the namespace refactor. 😱

keflavich avatar Nov 11 '20 14:11 keflavich

@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?

bsipocz avatar May 08 '23 12:05 bsipocz

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.

keflavich avatar May 08 '23 12:05 keflavich

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.

bsipocz avatar May 08 '23 12:05 bsipocz

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.

keflavich avatar May 08 '23 12:05 keflavich