Skip to content

Modifying astroquery/sdss package init file to use most current SDSS DR (as well as associated documentation) #2478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 11, 2022
Merged

Modifying astroquery/sdss package init file to use most current SDSS DR (as well as associated documentation) #2478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b2c0eb
SDSS module: set default data release to DR17
jsobeck Aug 2, 2022
32c7543
Slight change to sdss-associated documentation
jsobeck Aug 2, 2022
5f60c09
Minor updates for DR17
bsipocz Sep 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ sdss
- ``query_region()`` now does a cone search around the specified
coordinates. [#2477]

- The default data release has been changed to DR17. [#2478]



Infrastructure, Utility and Other Changes and Additions
-------------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion astroquery/sdss/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class Conf(_config.ConfigNamespace):
timeout = _config.ConfigItem(
60,
'Time limit for connecting to SDSS server.')
default_release = _config.ConfigItem(14, 'Default SDSS data release.')
default_release = _config.ConfigItem(17, 'Default SDSS data release.')


conf = Conf()
Expand Down
2 changes: 1 addition & 1 deletion astroquery/sdss/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
__all__ = ['SDSS', 'SDSSClass']
__doctest_skip__ = ['SDSSClass.*']


# Imaging pixelscale 0.396 arcsec
sdss_arcsec_per_pixel = 0.396 * u.arcsec / u.pixel


Expand Down
2 changes: 1 addition & 1 deletion astroquery/sdss/tests/test_sdss_remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,4 @@ def test_large_crossid(self, large_results):
# Regression test for #589

results = sdss.SDSS.query_crossid(large_results)
assert len(results) == 894
assert len(results) == 845
10 changes: 10 additions & 0 deletions docs/sdss/sdss.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@
SDSS Queries (`astroquery.sdss`)
********************************

Default Data Release
====================

The default data release is set to Data Release 17 (DR17), which is
the final data release of the Sloan Digital Sky Survey IV. DR17
contains new optical and infrared spectra from both Apache Point
Observatory and Las Campanas Observatory. Previously released
integral-field datacubes and maps, stellar library spectra, as well as
images, are also included in DR17. Users may select alternate DR's.
Copy link
Member

@eerovaher eerovaher Aug 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can a different data release be selected? Documentation is not helpful if it just says that something can be done, but neither describes nor provides examples how.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, first timer. In the core routine of the sdss package, the various query procedures allow for the data release parameter to be specified (with the default DR set initially). This has never been documented properly with examples (though seems straightforward for user specification). The easiest thing would be to remove the phrase now and then, take some time to generate an example. Sound alright?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that changing the data release is not documented is a shortcoming that should be fixed, but I would tend to agree that it would be beyond the scope of a pull request that simply updates the default. But perhaps it would be better to remove the entire section because only removing the last sentence might leave the wrong impression that DR17 is the only option.

Copy link
Contributor Author

@jsobeck jsobeck Aug 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chatted about this with @weaverba137. We believe that it is important to note what the default data release is and what it contains. This omission by astroquery is what lead to the initial issue request as users were confused as to why DR14 was employed by astroquery as opposed to the most recent DR from SDSS (DR17). The language can be massaged accordingly, but users do have some inherent responsibility to understand something about the database/dataset they are querying. If a user is employing SDSS data, there is a high probability that they understand that it has issued multiple DR's over the years.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the language @jsobeck is suggesting should remain, because it opens the door to astroquery eventually getting the functionality to access these other SDSS data sets.

More comments below.


Getting started
===============

Expand Down