Skip to content

add testpaths to setup.cfg pytest section #1137

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

Merged
merged 2 commits into from
Jan 23, 2021

Conversation

wholmgren
Copy link
Member

  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

I had been running into odd behavior with vscode no longer able to automatically discover pvlib tests and with strange errors when trying to run pytest from the root pvlib directory - sometimes I'd get numpy import errors in good environments and sometimes I'd see errors with old pvlib version numbers. But pytest pvlib worked just fine. It turns out that the presence of conda environments in benchmarks/ led to bad numpy imports and the presence of old pvlib versions in dist/ led to other issues. Adding these directories to the setup.cfg pytest section solved the issues, and I added a bunch of extra directories for safety.

@wholmgren wholmgren added this to the 0.9.0 milestone Jan 20, 2021
@wholmgren
Copy link
Member Author

@kanderso-nrel @mikofski can you merge this if you have no concerns with it? It feels like just a bit more than I should do without a second opinion.

@kandersolar kandersolar self-requested a review January 21, 2021 17:52
Copy link
Member

@kandersolar kandersolar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If all of our tests are in one place (/pvlib/tests), would it make more sense to tell pytest where to look with testpaths (docs) instead of where not to look with norecursedirs?

Also the pytest docs recommend not using setup.cfg except for very simple cases. So if at some point in the future we find ourselves looking at this config section again, might be worth switching to one of the recommended options.

@wholmgren
Copy link
Member Author

If all of our tests are in one place (/pvlib/tests), would it make more sense to tell pytest where to look with testpaths (docs) instead of where not to look with norecursedirs?

that is more clear and I think it might have been faster to collect too.

Also the pytest docs recommend not using setup.cfg except for very simple cases. So if at some point in the future we find ourselves looking at this config section again, might be worth switching to one of the recommended options.

I saw that too. I'm not sure what qualifies as too complicated for setup.cfg. Maybe we could condense a few configuration files into a pyproject.toml file.

@kandersolar
Copy link
Member

I think the config in the most recent commit qualifies as simple. No objections from me to merging, but I don't have benchmark envs handy so I didn't reproduce the issue locally to confirm the fix works.

@wholmgren wholmgren changed the title add norecursedirs to setup.cfg pytest section add testpaths to setup.cfg pytest section Jan 23, 2021
@wholmgren wholmgren merged commit 1385943 into pvlib:master Jan 23, 2021
@wholmgren wholmgren deleted the vscodepytest branch January 23, 2021 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants