astropy / astroquery

Functions and classes to access online data resources. Maintainers: @keflavich and @bsipocz and @ceb8
http://astroquery.readthedocs.org/en/latest/
BSD 3-Clause "New" or "Revised" License
706 stars 398 forks source link

MAINT/BUG: remove use of `**kwargs` in function signatures #2486

Open bsipocz opened 2 years ago

bsipocz commented 2 years ago

This is the extension of #2109 that is about the bug it causes in the mast module. Basically, we should be explicit and list all optional keyword parameters rather than blindly covering them in e.g. a **kwarg, which should only be used for wrappers that have too many options depending on the different backend.

Being explicit is 1) more useful for the users to see all the options in the signature/docstring, 2) less error-prone by ensuring ineffective keywords are not accepted (and misleading users that they are used while in fact, they have no effect whatsoever).

I'm attaching a checklist for all our current modules as this issue can be closed when it's systematically addressed in all of them (and explicit exceptions are made for the remaining usage of **kwargs).

eerovaher commented 2 years ago

The following shell command reveals which sub-directories of astroquery contain the phrase "**kwargs":

$ git grep  \*\*kwargs -- astroquery/*/ | cut -d / -f2 | uniq
alfalfa
alma
besancon
cadc
casda
cds
esa
esasky
eso
fermi
gama
gemini
heasarc
hips2fits
hitran
image_cutouts
imcce
ipac
jplhorizons
jplsbdb
lamda
linelists
magpis
mast
mpc
nasa_ads
nist
nrao
nvas
oac
ogle
sdss
simbad
skyview
splatalogue
svo_fps
template_module
ukidss
utils
vizier
vo_conesearch
wfau
xmatch

The ones that are not mentioned in the output above should be safe to tick off the checklist.

bsipocz commented 2 years ago

Thanks for the one liner. There are a few other **something in signatures, some may also need to be cleaned up, so I would generalize the above to use grep -E '\*\*[a-z]' instead.

(In practice that adds only these two to the list a modules, but when cleaning up the rest, it would be best to check out for those other usages, too:

astrometry_net open_exoplanet_catalogue)