Fix SDSS CrossID method not working for spec data and for DR17 (photo and spec) by nmcardoso · Pull Request #2258 · astropy/astroquery

nmcardoso · 2022-01-16T13:34:30Z

Sumary

This PR aims to fix the following errors in the astroquery.sdss.SDSS.query_crossid_async() method:

Fix documentation typo about default value of the radius param (bcef716)
Fix spec query not working for DR >= 12 (777f80b)
Fix photo and spec query not working for DR 17 (4fb7217)

Details about each fix

1. Documentation typo

Changed documentation to the right value of the default radius param

Commit: bcef716

2. Spec query for DR >= 12

Spec queries for DR >= 12 are resulting in a 500 (Internal Server Error) HTTP response
becouse the query string formed by this method is invalid when requesting columns of
SpecObj table. The query properly includes the spec columns but forgot the
JOIN statement with the SpecObj table.

Commit: 777f80b

3. Photo and spec query for DR 17

Both kind of queries to the DR 17 API results in a 500 (Internal Server Error) HTTP response
becouse this API requires parameters to be sent in the request body as a payload.
Passing the parans via data attribute instead of the params attribute of the
http client solves the issue, since previous data releases (DR >= 12) allows this approach too.

Commit: 4fb7217

Tests Results

All DR >= 12 where tested for photo and spec query.
All requests returned 200 status code and the method returned an correct astropy.table.table.Table instance.

Test code:

from astroquery.sdss import SDSS
from astropy import coordinates as coord
import numpy as np
from astropy import units as u
from astroquery import log
log.setLevel('TRACE')

pos = coord.SkyCoord(
  ra=np.array([359.970818400315, 0.1286587799259]), 
  dec=np.array([-1.21788544571165, -1.21302218898355]), 
  frame='icrs', 
  unit='deg'
)
result = SDSS.query_crossid(
  coordinates=pos, 
  photoobj_fields=['objID', 'ra', 'dec'],
  specobj_fields=['specObjID', 'z'], 
  data_release=17,
  radius=2*u.arcsec,
  cache=False
)

Note about DR < 12:
Queries in data realeases < 12 results in a 404 HTTP status code, before and after this changes.
Looks like services for old DRs have been removed

DR 12 (200 OK)

200 OK http://skyserver.sdss.org/dr12/en/tools/search/X_Results.aspx
Cache-Control: private
Content-Type: text/plain; charset=utf-8
Content-Encoding: gzip
Vary: Accept-Encoding
Server: Microsoft-IIS/8.5
Access-Control-Allow-Origin: *
Content-Disposition: attachment;filename="Skyserver_CrossID1/16/2022 1:11:46 PM.csv"
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET
Date: Sun, 16 Jan 2022 13:11:45 GMT
Content-Length: 322

#Table1
obj_id,objID,ra,dec,obj_id1,specObjID,z,type
obj_0,1237663275780276471,359.970818399119,-1.21788545489075,1237663275780276471,435757928612390912,0.1629147,GALAXY
obj_1,1237663275780341888,0.128658777722677,-1.21302220589001,1237663275780341888,435746658618206208,0.07535086,GALAXY

DR 13 (200 OK)

200 OK http://skyserver.sdss.org/dr13/en/tools/search/X_Results.aspx
Cache-Control: private
Content-Type: text/plain; charset=utf-8
Content-Encoding: gzip
Vary: Accept-Encoding
Server: Microsoft-IIS/8.5
Access-Control-Allow-Origin: *
Content-Disposition: attachment;filename="Skyserver_Radial1/16/2022 1:14:47 PM.csv"
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET
Date: Sun, 16 Jan 2022 13:14:47 GMT
Content-Length: 325

#Table1
obj_id,objID,ra,dec,obj_id1,specObjID,z,type
obj_0,1237663275780276471,359.970818400315,-1.21788544571165,1237663275780276471,435757928612390912,0.1629147,GALAXY
obj_1,1237663275780341888,0.1286587799259,-1.21302218898355,1237663275780341888,435746658618206208,0.07535086,GALAXY

DR 14 (200 OK)

200 OK http://skyserver.sdss.org/dr14/en/tools/search/X_Results.aspx
Cache-Control: private
Content-Type: text/plain; charset=utf-8
Content-Encoding: gzip
Vary: Accept-Encoding
Server: Microsoft-IIS/8.5
Access-Control-Allow-Origin: *
Content-Disposition: attachment;filename="Skyserver_SQL1/16/2022 1:15:18 PM.csv"
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET
Date: Sun, 16 Jan 2022 13:15:18 GMT
Content-Length: 325

#Table1
obj_id,objID,ra,dec,obj_id1,specObjID,z,type
obj_0,1237663275780276471,359.970818400315,-1.21788544571165,1237663275780276471,435757928612390912,0.1629147,GALAXY
obj_1,1237663275780341888,0.1286587799259,-1.21302218898355,1237663275780341888,435746658618206208,0.07535086,GALAXY

DR 15 (200 OK)

200 OK http://skyserver.sdss.org/dr15/en/tools/search/X_Results.aspx
Cache-Control: private
Content-Type: text/plain; charset=utf-8
Content-Encoding: gzip
Vary: Accept-Encoding
Server: Microsoft-IIS/8.5
Access-Control-Allow-Origin: *
Content-Disposition: attachment;filename="Skyserver_CrossID1/16/2022 1:15:42 PM.csv"
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET
Date: Sun, 16 Jan 2022 13:15:42 GMT
Content-Length: 325

#Table1
obj_id,objID,ra,dec,obj_id1,specObjID,z,type
obj_0,1237663275780276471,359.970818400315,-1.21788544571165,1237663275780276471,435757928612390912,0.1629147,GALAXY
obj_1,1237663275780341888,0.1286587799259,-1.21302218898355,1237663275780341888,435746658618206208,0.07535086,GALAXY

DR 16 (200 OK)

200 OK http://skyserver.sdss.org/dr16/en/tools/search/X_Results.aspx
Cache-Control: private
Content-Type: text/plain; charset=utf-8
Content-Encoding: gzip
Vary: Accept-Encoding
Server: Microsoft-IIS/8.5
Access-Control-Allow-Origin: *
Content-Disposition: attachment;filename="Skyserver_SQL1/16/2022 1:15:58 PM.csv"
X-AspNet-Version: 4.0.30319
X-Powered-By: ASP.NET
Date: Sun, 16 Jan 2022 13:15:57 GMT
Content-Length: 325

#Table1
obj_id,objID,ra,dec,obj_id1,specObjID,z,type
obj_0,1237663275780276471,359.970818400315,-1.21788544571165,1237663275780276471,435757928612390912,0.1629147,GALAXY
obj_1,1237663275780341888,0.1286587799259,-1.21302218898355,1237663275780341888,435746658618206208,0.07535086,GALAXY

DR 17 (200 OK)

200 OK http://skyserver.sdss.org/dr17/en/tools/search/X_Results.aspx
Transfer-Encoding: chunked
Content-Type: text/plain
Content-Encoding: gzip
Vary: Accept-Encoding
Server: Kestrel
Content-Disposition: attachment;filename="Skyserver_CrossID1/16/2022 1:06:29 PM.csv"
X-Powered-By: ASP.NET
Date: Sun, 16 Jan 2022 13:06:29 GMT

#Table1
obj_id,objID,ra,dec,obj_id1,specObjID,z,type
obj_0,1237663275780276471,359.970818400315,-1.21788544571165,1237663275780276471,435757928612390912,0.1629147,GALAXY
obj_1,1237663275780341888,0.1286587799259,-1.21302218898355,1237663275780341888,435746658618206208,0.07535086,GALAXY

codecov · 2022-01-16T13:41:04Z

Codecov Report

Merging #2258 (0eaa824) into main (a31ab86) will increase coverage by 0.07%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main    #2258      +/-   ##
==========================================
+ Coverage   62.60%   62.68%   +0.07%     
==========================================
  Files         131      131              
  Lines       16784    16818      +34     
==========================================
+ Hits        10508    10542      +34     
  Misses       6276     6276

Impacted Files	Coverage Δ
astroquery/sdss/core.py	`85.76% <75.00%> (-0.21%)`	⬇️
astroquery/esa/xmm_newton/core.py	`64.45% <0.00%> (+3.99%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a31ab86...0eaa824. Read the comment docs.

keflavich

Nice! Looks good to me. Thanks for the thorough writeup.

ceb8

I think this looks great! Would be nice to have an added test for the new option.

pep8speaks · 2022-01-18T21:18:11Z

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

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-01-18 21:21:49 UTC

ceb8 · 2022-01-18T21:52:04Z

Thanks @nmcardoso !

nmcardoso · 2022-01-18T23:40:44Z

@ceb8 😀👍

bsipocz · 2022-02-23T08:05:20Z

The test added here were never actually worked. It wasn't picked up by CI by design as remote tests are not run in CI, but they should have been checked before the merge. I'm opening a fix separately.

bsipocz · 2022-02-23T08:14:24Z

Actually, before doing a fix, I wonder how this ever worked for you @nmcardoso? As I actually run into the issue that the raw SQL doesn't work with the error Invalid column name 'objID, and the code that generates that part of the SQL doesn't seem to be changing with this PR.

bsipocz · 2022-02-23T09:04:40Z

The fixes and changelog is added for this in #2304

nmcardoso added 3 commits January 16, 2022 10:20

Fix documentation typo about default radius value

bcef716

Fix spec query not working for DR >= 12

777f80b

Fix photo and spec query not working for DR 17

4fb7217

keflavich added the sdss label Jan 18, 2022

keflavich approved these changes Jan 18, 2022

View reviewed changes

ceb8 reviewed Jan 18, 2022

View reviewed changes

Comment thread astroquery/sdss/core.py

sdss crossid tests with spectro paramas

fb84a65

fix PEP8 in last commit

0eaa824

ceb8 approved these changes Jan 18, 2022

View reviewed changes

ceb8 merged commit 091b987 into astropy:main Jan 18, 2022

bsipocz added this to the v0.4.6 milestone Jan 18, 2022

nmcardoso deleted the fix-query_crossid_async branch January 18, 2022 23:38

nmcardoso restored the fix-query_crossid_async branch January 18, 2022 23:38

bsipocz mentioned this pull request Feb 23, 2022

Fix remaining issues with spectra Sdss.query_crossid #2304

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix SDSS CrossID method not working for spec data and for DR17 (photo and spec)#2258

Fix SDSS CrossID method not working for spec data and for DR17 (photo and spec)#2258
ceb8 merged 5 commits into
astropy:mainfrom
nmcardoso:fix-query_crossid_async

nmcardoso commented Jan 16, 2022

Uh oh!

codecov Bot commented Jan 16, 2022 •

edited

Loading

Uh oh!

keflavich left a comment

Uh oh!

ceb8 left a comment

Uh oh!

Uh oh!

pep8speaks commented Jan 18, 2022 •

edited

Loading

Uh oh!

ceb8 commented Jan 18, 2022

Uh oh!

nmcardoso commented Jan 18, 2022

Uh oh!

bsipocz commented Feb 23, 2022

Uh oh!

bsipocz commented Feb 23, 2022

Uh oh!

bsipocz commented Feb 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

nmcardoso commented Jan 16, 2022

Sumary

Details about each fix

1. Documentation typo

2. Spec query for DR >= 12

3. Photo and spec query for DR 17

Tests Results

DR 12 (200 OK)

DR 13 (200 OK)

DR 14 (200 OK)

DR 15 (200 OK)

DR 16 (200 OK)

DR 17 (200 OK)

Uh oh!

codecov Bot commented Jan 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

keflavich left a comment

Choose a reason for hiding this comment

Uh oh!

ceb8 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pep8speaks commented Jan 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2022-01-18 21:21:49 UTC

Uh oh!

ceb8 commented Jan 18, 2022

Uh oh!

nmcardoso commented Jan 18, 2022

Uh oh!

bsipocz commented Feb 23, 2022

Uh oh!

bsipocz commented Feb 23, 2022

Uh oh!

bsipocz commented Feb 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Jan 16, 2022 •

edited

Loading

pep8speaks commented Jan 18, 2022 •

edited

Loading