Pass in resolver name to MAST query functions#3292
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3292 +/- ##
==========================================
+ Coverage 69.36% 69.51% +0.15%
==========================================
Files 232 232
Lines 19692 19739 +47
==========================================
+ Hits 13659 13722 +63
+ Misses 6033 6017 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bsipocz
left a comment
There was a problem hiding this comment.
We have a related test failure, please have a look into it:
>>> ned_loc = utils.resolve_object("jw100", resolver="NED")
...
E astroquery.exceptions.ResolverError: Could not resolve jw100 to a sky position using NED. Please try another resolver or set ``resolver=None`` to use the first compatible resolver.
|
Hmm, I can't reproduce this. I believe that changes were just deployed to our SANTA service, so you may have caught it while it was down? |
bsipocz
left a comment
There was a problem hiding this comment.
Minor comments about mandatory kwargs. Besides those this needs a rebase to resolve the changelog conflict
|
|
||
|
|
||
| def resolve_object(objectname): | ||
| def resolve_object(objectname, resolver=None, resolve_all=False): |
There was a problem hiding this comment.
Make these mandatory kwargs, here and the other methods, too.
| return coord | ||
|
|
||
|
|
||
| def parse_input_location(coordinates=None, objectname=None, resolver=None): |
There was a problem hiding this comment.
make all of these mandatory kwargs, too. I know it's breaking API, but it may prevent some buggy user code in the longer run
| resolver : str, List, optional | ||
| Name of resolver. Must be "NED" or "SIMBAD". This parameter is case-insensitive. |
There was a problem hiding this comment.
duplicate
| resolver : str, List, optional | |
| Name of resolver. Must be "NED" or "SIMBAD". This parameter is case-insensitive. |
| pass | ||
|
|
||
| def resolve_object(self, objectname): | ||
| def resolve_object(self, objectname, resolver=None, resolve_all=False): |
There was a problem hiding this comment.
| def resolve_object(self, objectname, resolver=None, resolve_all=False): | |
| def resolve_object(self, objectname, *, resolver=None, resolve_all=False): |
361a018 to
b7e2d79
Compare
This PR adds a
resolverparameter to various MAST query functions. This parameter allows users to specify what resolver they want to use when resolving object names into coordinates.It also adds
resolverandresolve_allparameters to theresolve_objectfunction. Ifresolve_allisTrue, thenresolve_objectwill return a dictionary where keys are resolver names and values are their resolved coordinates.I added test cases and updated our documentation to reflect these additions.
Also fixes some test failures brought on by numpy 2.0 changes and closes #3299