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

Ruff basic examples #2370

Merged
merged 3 commits into from
Oct 16, 2024
Merged

Ruff basic examples #2370

merged 3 commits into from
Oct 16, 2024

Conversation

EwoutH
Copy link
Member

@EwoutH EwoutH commented Oct 16, 2024

Run ruff and ruff format on the basic examples. Follow-up on #2365, as requested.

I fixed all the non-docstring errors, but there are still 56 docstring errors left:

  • D100 Missing docstring in public module — 14 occurrences
  • D102 Missing docstring in public method — 15 occurrences
  • D103 Missing docstring in public function — 9 occurrences
  • D107 Missing docstring in __init__ — 4 occurrences
  • D205 1 blank line required between summary line and description — 5 occurrences
  • D417 Missing argument descriptions in the docstring for __init__ — 3 occurrences
  • D101 Missing docstring in public class — 1 occurrence

For now we ignore them, but they can be unignored later if preferred.

Also update the pyproject.toml to enforce this (aeac384):

  • Only exclude examples/advanced from ruff, to start running ruff on the basic examples.
  • Ignore docstring errors in the example folder.

@EwoutH EwoutH added the example Changes the examples or adds to them. label Oct 16, 2024
@EwoutH EwoutH requested review from quaquel and Corvince October 16, 2024 10:23
Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🟢 -5.5% [-6.4%, -4.6%] 🟢 -5.3% [-5.7%, -5.0%]
BoltzmannWealth large 🔵 -0.8% [-1.2%, -0.5%] 🟢 -4.8% [-5.8%, -4.0%]
Schelling small 🔵 -1.4% [-1.9%, -0.9%] 🔵 -0.2% [-0.4%, +0.0%]
Schelling large 🔵 -1.9% [-2.9%, -0.6%] 🔵 -0.6% [-2.5%, +1.1%]
WolfSheep small 🔵 -0.5% [-0.7%, -0.3%] 🔵 -0.7% [-0.9%, -0.5%]
WolfSheep large 🔵 -1.0% [-2.0%, -0.3%] 🔵 -0.9% [-1.5%, -0.4%]
BoidFlockers small 🔵 -2.7% [-3.1%, -2.3%] 🔵 -0.1% [-0.7%, +0.5%]
BoidFlockers large 🔵 -3.5% [-4.3%, -2.8%] 🔵 -1.2% [-1.7%, -0.8%]

@quaquel
Copy link
Member

quaquel commented Oct 16, 2024

do you mind if I contribute to this directly by fixing the various outstanding issues?

@EwoutH
Copy link
Member Author

EwoutH commented Oct 16, 2024

Normally, no, but in this case I prefer merging this as is and doing follow-up PRs other issues.

This PR is now neat and atomic, and you can just remove that added line in pyproject.toml in a future PR to see all doc errors again.

Copy link
Contributor

@Corvince Corvince left a comment

Choose a reason for hiding this comment

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

Okay, I think it's already much better now, acknowledge not updating docstrings in this PR, but should be done in the future

@EwoutH EwoutH force-pushed the ruff_basic_examples branch from aeac384 to 5184c93 Compare October 16, 2024 12:33
Fix:
- N803 Argument name should be lowercase: 1 occurrence
- N806 Variable in function should be lowercase: 2 occurrences
- Only exclude examples/advanced from ruff, to start running ruff on the basic examples.
- Ignore docstring errors in the example folder.

Both can be revisited in the future.
@EwoutH EwoutH force-pushed the ruff_basic_examples branch from 5184c93 to ffe565b Compare October 16, 2024 12:35
@EwoutH EwoutH merged commit ffe565b into projectmesa:main Oct 16, 2024
10 of 12 checks passed
@EwoutH EwoutH deleted the ruff_basic_examples branch October 26, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
example Changes the examples or adds to them.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants