search

astropy / pytest-astropy

Metapackage for all the testing machinery used by the Astropy Project
BSD 3-Clause "New" or "Revised" License
4 stars 12 forks source link

Request: Move "-R" option from `pytest-remotedata` to wrapper `pytest-astropy` #54

Closed miurahr closed 1 year ago

miurahr commented 1 year ago

As reported on https://github.com/astropy/pytest-remotedata/issues/67 , "-R" option can be conflicted with pytest-leaks package that is also a pytest plugin.

As @bsipocz suggested, "-R" option is introduced for using on pytest-astropy so it can be moved, and then remove it from pytest-remotedata that can be combined with other plugins, which has also "-R" option.

pllim commented 1 year ago

Hmm, so users who use pytest-remotedata without pytest-astropy would not be able to use the -R option? I don't see the point. Might as well just remove -R altogether.

miurahr commented 1 year ago

My original request is to remove -R from pytest-remotedata. @bsipocz suggest "move" the option in original ticket. I post it here for responding the suggestion.

User can use long option, --remote-data when remove short option -R.

bsipocz commented 1 year ago

@pllim - given that this is/was a feature in the astropy testrunner rather than in the plugin itself, I don't see a big deal with moving that logic into pytest-astropy. Sure, it's not ideal, but a good enough middle-ground compromise, and I don't see pytest-leaks would do the change (I think our -R predates theirs, but as I said, it was in the runner level rather than on the plugin level, so the conflict hasn't come up early enough).

So, if there is a pair of PR that does the changes here and in pytest-remotedata I would approve and merge them and would tag new releases, but doing the change, too is pretty far low on my list (aka, I don't rule out that I'll get there eventually, but I can't give you an ETA for it)

pllim commented 1 year ago

Personally, I never use -R for remote-data, and it is not used anywhere in astropy automated CI.

Should we just go ahead and remove -R from pytest-remotedata and figure out the second part later if it is actually used?

bsipocz commented 1 year ago

What astropy core does has nothing to do with this, also, CI is not exactly the use case for convenience such as the short names.

If we start going into personal experience, I for one use -R all the time with the packages that are the power users for remote server access, e.g. astroquery and pyvo, and use it often enough for smaller usages elsewhere. None of these packages use pytest-leaks, thus moving the -R to the metapackage pytest-astropy is good compromise. But removing before it's been added to pytest-astropy is a clear no-go from me.