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

Bug fixes and cleanup to pass flake8 in CI #36

Merged
merged 6 commits into from
Jun 28, 2022

Conversation

brews
Copy link
Member

@brews brews commented Jun 23, 2022

We got automated tests up and running in CI with PR #35. Once it started to run flake8 immediately flagged climate_toolbox code for a number of reasons. This PR resolves those issues.

  • Add a basic flake8 configuration to setup.cfg so we focus on big and immediate problems.
  • I autoformatted all package and test code with black. This isn't required right now, but it's the fastest way to get minor formatting fixes so the code passes the flake8 check in CI.
  • Fix "undefined" error in climate_toolbox/utils/utils.py.
  • Fix several ambiguous star imports in climate_toolbox/io/io.py and tests/test_climate_toolbox.py.
  • Fix an issue with overwritten test functions in tests/test_climate_toolbox.py.
  • Dropped unused variable in load_bcsd() from climate_toolbox/io/io.py.

With these changes, flake8 passes the code on my local machine.

@brews brews self-assigned this Jun 23, 2022
@brews
Copy link
Member Author

brews commented Jun 23, 2022

This PR passes now the flake8 check in CI but fails when pytest is run. This is expected and progress because pytest wouldn't even run before this. That is, the fact that this is even running is good — even if it's failing.

Fixing the new issues with this pytest check will need to come in a later PR, likely at a later date.

@brews brews requested a review from delgadom June 23, 2022 19:30
@brews brews merged commit c01ace2 into ClimateImpactLab:master Jun 28, 2022
@brews brews deleted the flake8_cleanup branch June 28, 2022 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants