Navigation Menu

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

Test the consistency of requirements defined in different locations #1500

Merged
merged 30 commits into from Aug 16, 2022

Conversation

namurphy
Copy link
Member

This PR adds tests to check that the requirements defined in the {build,docs,extras,install,tests}.txt files are consistent with the requirements defined in setup.cfg and pyproject.toml. Turns out that we can use setuptools.config.read_configuration to read in setup.cfg, and toml.loads to read in pyproject.toml. Closes #1277.

One thing I needed to handle was that the tests and docs requirements in setup.cfg also include the extras requirements.

This PR also includes an explicit import plasmapy statement, so I'm wondering if #1447 could possibly be closed by this as well. I checked and didn't find any import plasmapy lines in any of our other tests, so this would be helpful to add.

@codecov
Copy link

codecov bot commented Mar 29, 2022

Codecov Report

Merging #1500 (771d408) into main (94d2abb) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1500      +/-   ##
==========================================
- Coverage   97.23%   97.22%   -0.01%     
==========================================
  Files          84       84              
  Lines        8016     8000      -16     
==========================================
- Hits         7794     7778      -16     
  Misses        222      222              
Impacted Files Coverage Δ
plasmapy/particles/ionization_state.py 94.06% <0.00%> (-0.14%) ⬇️
plasmapy/particles/particle_class.py 98.99% <0.00%> (-0.01%) ⬇️
plasmapy/particles/atomic.py 100.00% <0.00%> (ø)
plasmapy/particles/__init__.py 95.83% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@namurphy
Copy link
Member Author

namurphy commented Apr 5, 2022

  • Finish updating usage of toml to tomli, since they seem to have a different API.

@namurphy
Copy link
Member Author

namurphy commented Apr 5, 2022

@rocco8773's idea regarding the pre-commit test failure — try putting a restriction on which version of click can be used in additional dependencies in .pre-commit-config.yaml

@namurphy namurphy added status: ready for review PRs that are ready for code review testing Availability size: medium 30 ≤ changed lines < 100 labels Apr 12, 2022
@namurphy namurphy requested a review from a team April 13, 2022 14:45
plasmapy/tests/test_requirements.py Show resolved Hide resolved
plasmapy/tests/test_requirements.py Outdated Show resolved Hide resolved
plasmapy/tests/test_requirements.py Outdated Show resolved Hide resolved
Comment on lines +47 to +50
return {
prefix: read_requirements_txt_file(prefix, requirements_directory)
for prefix in requirements_prefixes
}
Copy link
Member

Choose a reason for hiding this comment

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

A point on design. I've been reading https://web.stanford.edu/~ouster/cgi-bin/book.php lately, and I can't help but notice that these few functions you've got here are rather shallow - they don't hide much complexity, if any. I get the fear of long functions, but I think these oneliners could be, well, inlined.

plasmapy/tests/test_requirements.py Outdated Show resolved Hide resolved
namurphy and others added 3 commits April 19, 2022 13:14
Co-authored-by: Dominik Stańczak <stanczakdominik@gmail.com>
Co-authored-by: Dominik Stańczak <stanczakdominik@gmail.com>
@namurphy namurphy added this to the 0.9.0 milestone Jun 28, 2022
Copy link
Member

@StanczakDominik StanczakDominik left a comment

Choose a reason for hiding this comment

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

I have one single suggestion that fixes the CI problem, then it's a speedy merge :) I've tested that it does work nicely on adding a surplus package to setup.cfg.

plasmapy/tests/test_requirements.py Outdated Show resolved Hide resolved
@StanczakDominik
Copy link
Member

pre-commit.ci autofix

pre-commit-ci bot and others added 2 commits August 16, 2022 16:24
Co-authored-by: Dominik Stańczak-Marikin <stanczakdominik@gmail.com>
@namurphy
Copy link
Member Author

pre-commit.ci autofix

@namurphy namurphy enabled auto-merge (squash) August 16, 2022 16:47
@namurphy namurphy merged commit 2fa07b6 into PlasmaPy:main Aug 16, 2022
@namurphy namurphy removed the status: ready for review PRs that are ready for code review label May 23, 2023
@namurphy namurphy deleted the test-requirements-consistency branch May 24, 2023 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: medium 30 ≤ changed lines < 100 testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test the consistency of requirements defined in multiple places
2 participants