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

Improve current ruff rules #1720

Merged
merged 4 commits into from
Jan 16, 2024
Merged

Improve current ruff rules #1720

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c5f346
improve current ruff rules
juanitorduz Jan 15, 2024
dbf5916
rm comments
juanitorduz Jan 15, 2024
493da8e
fix lengths
juanitorduz Jan 16, 2024
58c0bba
trim
juanitorduz Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions numpyro/infer/mcmc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,10 +723,10 @@ def print_summary(self, prob=0.9, exclude_deterministic=True):

def transfer_states_to_host(self):
"""
Reduce the memory footprint of collected samples by transfering them to the host device.
Reduce the memory footprint of collected samples by transfering them to the host device.
"""
self._states = device_get(self._states)
self._states_flat = device_get(self._states_flat)
self._states_flat = device_get(self._states_flat)

def __getstate__(self):
state = self.__dict__.copy()
Expand Down
6 changes: 2 additions & 4 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,11 @@ exclude = [
]

# Same as Black.
line-length = 88
line-length = 120
Copy link
Member

Choose a reason for hiding this comment

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

I guess we prefer 88 here

Copy link
Contributor Author

@juanitorduz juanitorduz Jan 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think before we started the migration to ruff we had 120 (see original PR).
I can of course change it to 88. I might need to format some code a bit if I remember correctly. Is that ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I set 88 I get Found 821 errors (this comes from the 120 rule I mentioned). I could either keep 120 or set it to 88 and ignore the E501 rule as it is very hard to fix 821 errors :)

Copy link
Member

Choose a reason for hiding this comment

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

Previously, we allowed doc length 120 and code length 88. Do you think that we can maintain that behavior? If we set 120 here, will long code be formatted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we set 120 there is not refactor needed at all. I guess we can simply add 88 and ignore the rule for docstrings

Copy link
Member

Choose a reason for hiding this comment

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

Seems like

[tool.ruff.lint.pycodestyle]
max-line-length = 120

does the job

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed! Thanks 💪 ! See 493da8e

indent-width = 4

[tool.ruff.lint]
# Enable Pyflakes (`F`) and a subset of the pycodestyle (`E`) codes by default.
# We also add isort.
select = ["E4", "E7", "E9", "F", "I"]
select = ["E", "F", "I", "W"]
ignore = ["E203"]

# Allow fix for all enabled rules (when `--fix`) is provided.
Expand Down
Loading