astropy / astropy/astroquery

Unify row_limit

Open
#3,424 2 comments 0 reactions 0 assignees View on GitHub
cleanup question
Dominant language
Python
Stars
791
Forks
451
Avg merge
1d 3h
Merged PRs (30d)
4

Description

https://github.com/search?q=repo%3Aastropy%2Fastroquery+row_limit&type=code

`ROW_LIMIT` is used in SIMBAD, Vizier docs, vizier, ESO (for the moment), gaia, Euclid, IRSA's docs,

but `row_limit` is used in IRSA, esasky, mpc, vizier

The case difference is not consistent between all of these. I am not sure there was good motivation for the all-caps variables. Maybe, but if not, let's change all of them to `row_limit` everywhere.

More serious a problem, several modules use a completely different keyword to specify row limit:
[`maxrec`](https://github.com/search?q=repo%3Aastropy%2Fastroquery%20maxrec&type=code):
* heasarc
* cadc
* mocserver
* alma

`maxrecords`:
* mast

I'm not sure if there are other hidden ones, but I'd like to unify these at the user-facing level. TAP uses maxrec, so for most use cases, row_limit can just be passed in to TAP modules as maxrec.

I assume the rest of the modules that have none of these simply don't have a mechanism to limit query length.

In [this discussion](https://github.com/astropy/astroquery/pull/3339/files#r2244155474), I learned that there are separate `top` and `maxrec` concepts. The former lets users downselect in a TAP query based on the top N rows, while `maxrec` is just a hard cap on what gets returned and doesn't guarantee any logical order, if I understand correctly. My take is that `row_limit` is much closer to `maxrec`, but `top` is more valuable and should be the default when supported - as long as `top` shares the effect of limiting the amount of data sent. Generally the use case I see for these limits is running small test queries before running big production queries, which means the actual content of the small query is secondary to it being quick; the corollary is that if `top` takes longer than `maxrec` (because it has to do an additional sort), it should _not_ be the default option.

Contributor guide

Open the contributing guide

Research direction

Start by auditing the listed SIMBAD, Vizier, ESO, Gaia, Euclid, IRSA, ESASky, MPC, HEASARC, CADC, MOCServer, ALMA, and MAST modules, then compare their row-limit parameters with TAP's maxrec and top behavior. Done means the user-facing row-limit API is consistently defined across these modules without conflating maxrec and top.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.