Add NADC LAMOST module (astroquery.nadc.lamost) by SunnyHina · Pull Request #3666 · astropy/astroquery

SunnyHina · 2026-09-23T09:25:44Z

Add astroquery.nadc.lamost to access the LAMOST spectroscopic survey archive through the NADC OpenAPI.

The module provides cone searches, SQL and structured catalog queries, release and observation metadata, and downloads of catalog products and low- and medium-resolution spectra. It supports configurable authentication, typed Astropy tables, and readers for LAMOST FITS spectra.

Documentation includes query and download examples and a Ca II H&K activity-index example. Tests cover query validation, response parsing, configuration, downloads, spectrum readers, and the activity calculation. SQL requests use GET and remain subject to server URL-length limits.

Validation:

python -m pytest astroquery/nadc docs/nadc --remote-data=any -p no:cacheprovider -ra
465 passed, 2 skipped

All 17 remote tests and the documentation's remote examples passed. The two skipped doctests are examples explicitly marked +SKIP.

Check sort order and duplicate output columns for both request paths. Reject contains constraints without a value with InvalidQueryError.

Use the default filename when a Content-Disposition filename resolves to an empty name, a dot or a parent directory. Accept an empty save_dir as the current directory.

Strip optional token quotes consistently across configuration sources and report unreadable files with InputWarning.

Avoid duplicate columns in empty cone-search results. Repair VOTable FIELD elements with missing datatypes, validate table metadata and reject multidimensional scalar columns. Handle empty FITS EXTNAME cards.

Reset the shared client's URL, timeout, release and sub-version so local astroquery.cfg settings do not affect offline tests.

Convert string, numeric and boolean columns in batches while preserving masks and retaining element-wise conversion for object columns. Stream VOTable inspection when checking for empty integer cells.

Update the documentation index and changelog, enable remote examples, and document FITS header repair when downloading the example spectrum. Expose the spectrum download cache option and fix API references.

codecov · 2026-09-23T09:30:33Z

Codecov Report

❌ Patch coverage is 83.33333% with 57 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.76%. Comparing base (4c16adc) to head (987490c).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
astroquery/nadc/lamost/_sql.py	77.71%	37 Missing ⚠️
astroquery/nadc/lamost/_utils.py	85.92%	19 Missing ⚠️
astroquery/nadc/lamost/_response_utils.py	96.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3666      +/-   ##
==========================================
+ Coverage   73.69%   74.76%   +1.06%     
==========================================
  Files         230      235       +5     
  Lines       21551    22957    +1406     
==========================================
+ Hits        15882    17163    +1281     
- Misses       5669     5794     +125

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

keflavich

I've started a review of the documentation. I'm about to line 350, but I want to post the partial review to acknowledge: thanks for the contribution! I'll want to complete the docs review and then at least skim over the tests and structure to make sure it conforms appropriately to the astroquery standards, but broadly this looks good and is a contribution I'm excited to have.

keflavich · 2026-09-23T14:22:52Z

+Run file-writing examples in a working directory reserved for this tutorial.
+Examples with ``overwrite=True`` replace their named output files on reruns.


This should not be a tutorial, it should be documentation; a tutorial is valuable, but needs to go elsewhere.

Removed the tutorial-style instructions from the introduction.

keflavich · 2026-09-23T14:23:27Z

+The imported ``Lamost`` object is an instance created at import time. Changing
+``conf`` does not update existing instances; create a new instance after
+changing configuration:


These are all astroquery default behaviors. It's ok to add some of them but you don't need to explain them at this level of detail.

Shortened the configuration explanation and kept the example.

keflavich · 2026-09-23T14:26:09Z

+  >>> from astroquery.nadc.lamost import LamostClass, conf
+  >>> with conf.set_temp('timeout', 120):
+  ...     lamost = LamostClass(token='', data_release='dr10', sub_version='v2.0')
+  >>> lamost.TIMEOUT


I like this example.

keflavich · 2026-09-23T16:27:28Z

+
+  >>> conf.token = 'your-token'  # doctest: +SKIP
+  >>> authenticated = LamostClass()  # doctest: +SKIP
+  >>> configured = LamostClass(pylamost_config='~/pylamost.ini')  # doctest: +SKIP


will this automatically go through os.path.expanduser?

Yes. _get_config() calls os.path.expanduser(os.fspath(config_file)) before checking or opening the file.

keflavich · 2026-09-23T16:32:37Z

+``query_region`` requests CSV by default. Some archive VOTable
+responses declare string columns too short for their data; the client raises
+``TableParseError`` instead of returning truncated
+identifiers when VOTable is explicitly requested.


I'm not sure I understand this exceptional case. Is it a common one that users should worry about?

The LAMOST API has two issues with its responses.

Some VOTables declare string fields too short for the values they contain, which can truncate identifiers during parsing. A limited workaround that widens those fields before parsing TABLEDATA preserves the original values in the tested samples. I haven't added it to the PR yet, and I don't have a timeline for a server-side fix. Would you be OK with adding this workaround?

Some endpoints also return a different format from the one requested—for example, VOTable instead of CSV. The client already detects the actual response format and handles that case. HTML result pages are still reported as errors.

yes, the workaround is fine; this clarification is helpful and may be worth noting in the docs as, presumably, we'll eventually remove the workaround once server-side fixes happen

keflavich · 2026-09-23T16:34:53Z

+CSV avoids the known VOTable format problem on endpoints that honor the
+format request. It does not establish that the service returned every match
+for any release or search size. A single query
+does not automatically retrieve additional pages.


This could use more explanation. What is the "known VOTable format problem on endpoints that honor the format request" ? Is that the TableParseError note above?

What does "additional pages" mean? Is the return sometimes paged, and not necessarily complete?

Yes, the format issue refers to the same problem mentioned above. I'll separate that explanation from pagination.
The 100-row default is set by the client; it isn't a general limit imposed by the API. In one test with 201 matches, leaving out the API's paging parameters returned all 201 rows.

A bounded page size can keep responses smaller, but it doesn't necessarily make the database query cheaper. I'm wondering whether the current default is useful enough here.

One option would be to support max_rows='all' to retrieve all matching rows, and possibly make that the default. Would you prefer that, or keep a bounded default and make fetching everything an explicit choice? Neither option is implemented yet.

A bounded default is the right choice. I think supporting max_rows='all' is a good idea too, if it isn't going to cause major problems, but certainly the default should be less data transferred.

My comment here was primarily one of clarification: just add your explanation to the docs.

Co-authored-by: Adam Ginsburg <keflavich@gmail.com>

SunnyHina added 11 commits September 23, 2026 08:49

Add NADC LAMOST client, authentication and typed table parsing

85e0d12

Add LAMOST catalog queries, metadata and downloads

c233fa4

Add LAMOST single-spectrum FITS readers

fdb8efa

Document LAMOST queries and spectral analysis

faca73e

Validate structured query inputs before sending requests

309bac8

Check sort order and duplicate output columns for both request paths. Reject contains constraints without a value with InvalidQueryError.

Handle invalid catalog download filenames

2270f1c

Use the default filename when a Content-Disposition filename resolves to an empty name, a dot or a parent directory. Accept an empty save_dir as the current directory.

Read pylamost configuration as UTF-8

296e7ad

Strip optional token quotes consistently across configuration sources and report unreadable files with InputWarning.

Fix empty results and metadata parsing

8db3fe0

Avoid duplicate columns in empty cone-search results. Repair VOTable FIELD elements with missing datatypes, validate table metadata and reject multidimensional scalar columns. Handle empty FITS EXTNAME cards.

Isolate LAMOST unit tests from local configuration

be065a1

Reset the shared client's URL, timeout, release and sub-version so local astroquery.cfg settings do not affect offline tests.

Use array operations for typed column conversion

14ff70a

Convert string, numeric and boolean columns in batches while preserving masks and retaining element-wise conversion for object columns. Stream VOTable inspection when checking for empty integer cells.

Align LAMOST documentation with astroquery conventions

4ce9027

Update the documentation index and changelog, enable remote examples, and document FITS header repair when downloading the example spectrum. Expose the spectrum download cache option and fix API references.

Reference upstream PR in LAMOST changelog

3dd9310

keflavich requested changes Sep 23, 2026

View reviewed changes

SunnyHina and others added 2 commits September 24, 2026 14:43

Remove redundant dependency notes

925ab9d

Co-authored-by: Adam Ginsburg <keflavich@gmail.com>

Simplify LAMOST documentation introduction and configuration

987490c

		Run file-writing examples in a working directory reserved for this tutorial.
		Examples with ``overwrite=True`` replace their named output files on reruns.

Uh oh!

Conversation

SunnyHina commented Sep 23, 2026

Uh oh!

codecov Bot commented Sep 23, 2026 • 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 23, 2026 •

edited

Loading