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.

Sign up for GitHub

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

Jump to bottom

Enforce stricter checks (ruff, mypy, and pylint) #55

Merged
merged 27 commits into from
Dec 13, 2024
Merged

Enforce stricter checks (ruff, mypy, and pylint) #55

merged 27 commits into from
Dec 13, 2024

Conversation

mcocdawc
Copy link
Contributor

@mcocdawc mcocdawc commented Dec 10, 2024 •
edited
Loading

  • fixed a few remaining import issues
  • this fixed all current mypy errors
  • enforce ruff check as part of the test-suite and as part of the commit hook
  • enforce a few pylint checks as well that cannot be done by ruff
  • enforce mypy as part of the test-suite, legacy files are explicitly black-listed in the mypy.ini. This allows to gradually switch on stricter type checks for older files and enforces it for newer ones.
  • Explicitly only support python >= 3.10
  • updated the requirements file for the test suite
  • updated installation instructions
  • refactored kbe.pfrag update_heff to not return anything since it has side-effects

@mcocdawc mcocdawc changed the title Also test mypy WIP: Also test mypy Dec 10, 2024
@mcocdawc mcocdawc force-pushed the also_test_mypy branch 2 times, most recently from 50bc627 to b22cb14 Compare December 12, 2024 18:14
@mcocdawc
better typing and fixed a minor bug for cubegen
168470a
@mcocdawc
explicitly fail when lo_method == "iao" and iao_val_core
3e833ed 
this automatically fixes a non-resolved import as well
@mcocdawc
kbe should use the molbe be_func and be_func_parallel
6309fcb 
this fixes the last wrong import
@mcocdawc
fixed mypy of scratch manager
c3a7280
@mcocdawc
fixed molbe_io_fcidump_test
9bf111f
@mcocdawc
improved mypy testing
3645ae1
@mcocdawc
fixed a wrong import
8d74c81
@mcocdawc
more explicit blacklist for tests
a8f687f
@mcocdawc
enable the test for mypy
ca5d111
@mcocdawc
moved write_cube to shared.io
06296dc 
this fixes a circular import of molbe.BE
molbe.BE depends on shared/helper, so we cannot import it in
shared/helper.py
@mcocdawc
fixed import in test
5b9c13e
@mcocdawc
bumped up python versions
8540049
@mcocdawc
changed quemb unittest to install pytest earlier
fbbcef8
@mcocdawc
actually also install pytest for test :-D
1cfdd0b
@mcocdawc
fixed typing issues in tests
fce077b
@mcocdawc
do explicit blacklist in mypy.ini
9eccd27
@mcocdawc
refactored kbe update_heff
64d696e
@mcocdawc
updated doc and removed some unnecessary commands
b8a6782 
- everythying uses `pip install .` now
- take out some not-anymore-needed conda specific installation commands
- take out PYTHONPATH fiddling
@mcocdawc
enforce checks for ruff check and pylint
0769f9a 
pylint only checks E0401,R0401,E0611 because the actually require
importing python code which `ruff` does not do
@mcocdawc
fixed comments in build yaml files
133e01c
@mcocdawc
made mypy testing even more beautiful
d2b9d50
@mcocdawc
fixed small type error in os.environ.get
2f55fba
@mcocdawc
fixed unsorted imports
03048d1
@mscho527 mscho527 removed their request for review December 13, 2024 15:35
@mcocdawc mcocdawc changed the title WIP: Also test mypy Enforce stricter checks (ruff, mypy, and pylint) Dec 13, 2024
@mcocdawc mcocdawc mentioned this pull request Dec 13, 2024
Merged
lweisburn
lweisburn reviewed Dec 13, 2024
View reviewed changes
README.md Show resolved Hide resolved
lweisburn
lweisburn reviewed Dec 13, 2024
View reviewed changes
Copy link
Contributor

@lweisburn lweisburn left a comment

Choose a reason for hiding this comment

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

Looks good to me!

One TODO: we will have to add SHCI instructions in the readme, once we review that part of the code. We will have to choose/test whether we keep it as an optional install, like Wannier90

ShaunWeatherly
ShaunWeatherly reviewed Dec 13, 2024
View reviewed changes
Copy link
Contributor

@ShaunWeatherly ShaunWeatherly left a comment

Choose a reason for hiding this comment

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

This looks good, just a couple of questions to be answered and then this is ready to be merged.

src/quemb/kbe/pfrag.py Outdated Show resolved Hide resolved
tests/static_analysis_requirements.txt Show resolved Hide resolved
@mcocdawc mcocdawc merged commit 875d76d into main Dec 13, 2024
4 checks passed
@mcocdawc mcocdawc deleted the also_test_mypy branch December 13, 2024 17:46
@mcocdawc mcocdawc mentioned this pull request Dec 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShaunWeatherly ShaunWeatherly ShaunWeatherly approved these changes

@lweisburn lweisburn lweisburn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mcocdawc @lweisburn @ShaunWeatherly