Update Spectra URLs#2214
Conversation
keflavich
left a comment
There was a problem hiding this comment.
Can we add a remote test for this, perhaps?
|
I'll see what I can do. |
Codecov Report
@@ Coverage Diff @@
## main #2214 +/- ##
=======================================
Coverage 61.80% 61.81%
=======================================
Files 129 129
Lines 16288 16295 +7
=======================================
+ Hits 10067 10072 +5
- Misses 6221 6223 +2
Continue to review full report at Codecov.
|
|
I've updated a few of the tests, but I don't have easy access to a Python 3.[789] environment at this moment, so I can't actually run a |
bsipocz
left a comment
There was a problem hiding this comment.
This will need a changelog.
Running the remote tests returns errors for DRs: 8, 9, 11
It's great that those errors are recovered, but otoh the newly added test extension don't really test the fix, they are just as failing as before this patch.
However, the patch indeed fixes the example in the issue comment, #1731 (comment). What about adding that one as a regression test?
|
There is still a problem to load spectra from DR16 data release since it needs to add '/lite' of '/full' in the link address which is not supported by previous data releases. Also, it seems that you do not need to explicitly provide {instrument} within the format of the link since 'sdss' is allowable by server. Otherwise to download dr16 spectra it should be 'eboss' instead of 'boss'! |
|
@balashev, this PR does account for the |
|
@bsipocz, can you show me an example of the failure error messages for DR8, 9? DR11 should not actually be included in the remote data test suite since it does not have any SkyServer support. |
|
@bsipocz, actually, I found a minor bug in the |
It returned those obscured ValueErrors, I'll be able to dig up the actual server responses a bit later and will dump it in a gist. |
It still produce invalid link address for dr16. It seems it should be |
|
@weaverba137: For DR9 it actually passes now with these warnings: And thus as that None is passed in to the next command of [edit]: sorry, the test doesn't even get to this second command as it fails on the Table assert in the line above |
|
@balashev - It seems to be working for DR16: |
|
@bsipocz: And it is not working for dr16 plates, e.g.:
|
|
This branch doesn't resolve for me the issue with the wrong usage of The very last line returns
Am I doing something wrong? Am I downloading the wrong branch/version/pull/patch? |
|
@Nestak2 - you're installing the wrong branch. I would suggest to simply wait until this is merged and then install the dev version from pypi. |
|
@bsipocz, thank you for the test outputs, I'm able to reproduce now at least, and will dig deeper. |
|
@bsipocz, I think I found the problems for DR8 & 9.
|
|
I'll fix the conflict with |
b6c0b17 to
44c2780
Compare
|
Could you also add the line from #2214 (comment) as a test given that slipped through originally? |
|
This line, right? |
|
And there is one more (a new) failure: |
|
OK, I think I've got both of those in, please test. |
|
Oops, caught one more bug in one of the tests. |
I have only used it for simple editing. I never tried to run the test suite on it. Sorry. 😬 |
bsipocz
left a comment
There was a problem hiding this comment.
All remotes pass now with previously failing examples included in the test suite.
|
Thanks @weaverba137! |
|
@bsipocz Thanks for the previous answer! Is now the new version/branch/patch available for use? How do I "install the dev version from pypi". I find no information to that in google, can you provide me with a code snippet? Sorry, I am a simple astrophysics PhD student, that can code in Python, but has no idea about IT, github, version control etc. What I can successfully do in my google colab notebook is: And this runs well as long as I have Is the error fixable with "install the dev version from pypi" or is the error independent? |
|
Regardless of PyPI pre-release or not, you can install the dev version like this:
|
|
It's now on pypi, too, so the a pip install with the |
|
I am not familiar with this remote service, but you might want to look at the install log carefully to make sure you really installed the dev version that you think you are installing. If you are using a Python environment manager like conda, try it on a new environment. |
|
p.s. Also make sure you clear the cache, just in case. |
|
@pllim Thanks, I will try out your suggestions! The code from above is actually on a completely clean conda environment. Could you do me the favor and check if my code from above throws an error for you, too? It should be just as simple as copy-paste-run (and of course download the current version of the repo, if you haven't yet). Tnx |
|
I can reproduce the error: from astropy import coordinates as coords
from astroquery.sdss import SDSS
res = SDSS.query_sql("select top 101 z, ra, dec, bestObjID from specObj where class = 'galaxy' and programname='eboss'")
co = coords.SkyCoord(ra=res['ra'], dec=res['dec'], unit='deg')
table_spec = SDSS.get_spectra(co)yields (I used 101 and 100 to avoid risk of caching confusing me) |
|
Sounds like you should open a new issue and ping SDSS developers. |
This PR: