Add example of how to get non-public (proprietary) data#3294
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3294 +/- ##
==========================================
+ Coverage 69.36% 69.47% +0.11%
==========================================
Files 232 232
Lines 19692 19708 +16
==========================================
+ Hits 13659 13692 +33
+ Misses 6033 6016 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| >>> # Now query including proprietary data | ||
| >>> proprietary_data = alma.query(payload=dict(project_code='2017.1.01355.L', public_data=False)) | ||
| >>> # Or restrict to only proprietary data | ||
| >>> only_proprietary = alma.query_region('W51', radius=25*u.arcmin, public_data=False) |
There was a problem hiding this comment.
noting that the query_region function takes public not public_data as the keyword. The issue is correct but the doc change isn't.
There was a problem hiding this comment.
thanks for catching that, it was indeed the problem. It comes from Alma.help() showing public_data, not public.
|
Right, it was user error: In [27]: len(Alma.query_region('W51', radius=25*u.arcmin, public=False))
Out[27]: 66
In [28]: len(Alma.query_region('W51', radius=25*u.arcmin, public=True))
Out[28]: 376
In [29]: len(Alma.query_region('W51', radius=25*u.arcmin, public=None, public_data=False))
Out[29]: 66
In [30]: len(Alma.query_region('W51', radius=25*u.arcmin, public=None, public_data=True))
Out[30]: 376but |
| if science is not None: | ||
| payload['science_observation'] = science | ||
| if public is not None: | ||
| if 'public_data' in kwargs: |
There was a problem hiding this comment.
This is why I really dislike **kwargs, it swallows stuff and creates inconsistencies like this almost duplicate of kwargs with the same functionality.
There was a problem hiding this comment.
I hear you, but I greatly prefer this over query(payload={'public_data' : True}). I find it really clunky & annoying to specify keywords as dicts.
The redundancy here is almost certainly my fault: I told @andamian that he should preserve as much of the old API as possible, so this is (probably, I'm going from memory here) an API backward-compatibility-driven choice.
There was a problem hiding this comment.
Redundancy was an agreed tradeoff to make the transition from the Web form to astroquery easier. Do you think it's time to remove public_data from the help to avoid further confusion?
There was a problem hiding this comment.
no, I think the redundancy is important here, we just needed the documentation to be clear
| >>> # First login to access proprietary data | ||
| >>> alma.login("username") # Will prompt for password | ||
| >>> # Now query including proprietary data | ||
| >>> proprietary_data = alma.query(payload=dict(project_code='2017.1.01355.L', public=False)) |
There was a problem hiding this comment.
Isn't it public=None to include both public and proprietary?
There was a problem hiding this comment.
yes, this is definitely a mistake
| >>> proprietary_data = alma.query(payload=dict(project_code='2017.1.01355.L', public=False)) | ||
| >>> # Or restrict to only proprietary data | ||
| >>> # include both public and proprietary data using public=None | ||
| >>> proprietary_data = alma.query(payload=dict(project_code='2017.1.01355.L', public=None)) |
There was a problem hiding this comment.
I forgot what happens for public=None and unauthenticated user. Returns only public data or fails?
bsipocz
left a comment
There was a problem hiding this comment.
Thanks @keflavich and @andamian for the review!
cc @andamian
@low-sky noted that there was no documentation explaining how to get proprietary data.
This example shows technically how to do it, but it won't actually do anything useful.
by contrast, in other regions, it does work
so @andamian any idea what's going on here? Why is it region-dependent?