Skip to content
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.

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

toplevel pytest not working #130

Closed
sbailey opened this issue May 10, 2023 · 2 comments
Closed

toplevel pytest not working #130

sbailey opened this issue May 10, 2023 · 2 comments

Comments

@sbailey
Copy link
Contributor

sbailey commented May 10, 2023

We're trying to move desi-related repos from the deprecated "python setup.py test" to "pytest". For the specsim package, "pytest specsim/tests" works, but if you just run "pytest" at the top level it tries to pickup things in non-test directories like ah_bootstrap.py and specsim/atmosphere.py .

Identify why pytest is picking up these non-test files, and try to reorganize so that "pytest" at the top level works.

Context: although running "pytest specsim/tests" is fine, it is a pain to have to remember which repos require which subdirectories to be specified and it would be preferable for all our repos to work with "pytest" alone without caveats, just like they used to all work with "python setup.py test".

@weaverba137 this is an example of what we discussed on Monday, where top-level "pytest" doesn't work correctly.

@weaverba137
Copy link
Member

This is a special case in a couple of ways, because specsim has historically followed the astropy-affiliated package pattern. That pattern includes built-in pytest configuration that is not included in other DESI packages.

In the case of specsim/atmosphere.py, the astropy-affiliated configuration enables doctest_plus, which runs tests on any examples in the package. There's an example on Line 28. Now I happen to think that testing examples is a good thing, but it's not something we've normally enforced in DESI.

I'm a little less clear on 'non-test directories like ah_bootstrap.py', because ah_bootstrap.py is a file, not a directory. It would be useful to see an actual test result showing every unexpected file or directory. In any case, the Astropy community no longer supports astropy_helpers, and has even deprecated their own package template in favor of much simpler packaging guidelines (which are actually more compatible with DESI's standard pattern).

The developers of this package need to make a definitive decision: fully update this package to the latest astropy-affiliated package patterns, or abandon astropy-affiliated status altogether and move towards a DESI-pattern package. In the latter case, a wider sample of the DESI collaboration would be able to provide support for this package.

@weaverba137
Copy link
Member

I believe this is now addressed by #136. Note however that in the course of updating the package infrastructure, I discovered a previously undeclared dependency on desimodel, which also brings in a dependency on desiutil. If this is not the case, please reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants