Skip to content

Conversation

@kafitzgerald
Copy link
Collaborator

Closes #268

This PR moves all Ruff configuration to pyproject.toml, adds pre-commit.ci (to make linting/formatting more visible in CI and streamlined w/ pre-commit), and applies a bunch of linting and formatting fixes.

I suspect we'll also want to remove the Ruff checks from the testing workflow in CI, but left those for now mostly for reference while we norm on configuration options (currently this varies between CI and the other configs).

Setting this as "draft" until we can find a convenient time to discuss. I know there's some other stuff pending at the moment and this will create a bunch of conflicts. Will be nice to address though and should help moving forward.

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 8.97833% with 1176 lines in your changes missing coverage. Please review.
✅ Project coverage is 14.36%. Comparing base (d1350d7) to head (3faac72).

Files with missing lines Patch % Lines
credit/models/crossformer_downscaling.py 2.85% 102 Missing ⚠️
credit/parser.py 21.60% 98 Missing ⚠️
credit/data.py 0.00% 77 Missing ⚠️
credit/skebs.py 11.76% 75 Missing ⚠️
credit/datasets/wrf_singlestep.py 0.00% 63 Missing ⚠️
credit/datasets/om4_multistep_batcher.py 0.00% 55 Missing ⚠️
credit/datasets/era5_multistep_batcher.py 0.00% 45 Missing ⚠️
credit/datasets/sequential_multistep.py 0.00% 38 Missing ⚠️
credit/metrics.py 0.00% 36 Missing ⚠️
credit/interp.py 26.66% 33 Missing ⚠️
... and 56 more
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #269   +/-   ##
=======================================
  Coverage   14.36%   14.36%           
=======================================
  Files         111      111           
  Lines       18020    18024    +4     
  Branches     3070     3070           
=======================================
+ Hits         2589     2590    +1     
- Misses      15194    15197    +3     
  Partials      237      237           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@djgagne
Copy link
Collaborator

djgagne commented Jan 12, 2026

I am fine with the changes in general. One question I have is what the default number of characters per line should be? Currently it's set at 180, but if we're going to be reformatting a bunch of existing code, should we shrink this back down to 120 or 80 to be PEP 8 compliant or leave it longer and let people use line wrapping or scrolling in their preferred text editor to deal with longer lines?

More vertically oriented code is easier to stack side by side and to navigate through in command line text editors like vim, but it does sometimes result in some weird formatting choices by the linter. Thoughts?

@kafitzgerald
Copy link
Collaborator Author

kafitzgerald commented Jan 13, 2026

I am fine with the changes in general. One question I have is what the default number of characters per line should be? Currently it's set at 180, but if we're going to be reformatting a bunch of existing code, should we shrink this back down to 120 or 80 to be PEP 8 compliant or leave it longer and let people use line wrapping or scrolling in their preferred text editor to deal with longer lines?

More vertically oriented code is easier to stack side by side and to navigate through in command line text editors like vim, but it does sometimes result in some weird formatting choices by the linter. Thoughts?

Thanks for raising this. I agree it'd be good to discuss.

The 180 comes from the Ruff configuration in the pyproject.toml (which doesn't seem to have been enforced previously, but was there already).

120 is a pretty common default for a lot of text editors, but a lot of (maybe most) Python projects that I'm aware of stick with the default of 88 from Ruff/Black so slightly above PEP8's 79. I've also seen 100 and 120, but less frequently and rarely over that.

I think my preference would be to move to the Ruff default, but I'd like to hear other's thoughts on this as well though (especially if folks have strong opinions).

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

Successfully merging this pull request may close these issues.

pre-commit is failing

3 participants