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

Enable pylint and other linting rules in ruff #417

Closed
wants to merge 5 commits into from

Conversation

markbandstra
Copy link
Member

This PR enables the following rulesets in ruff:

  • SIM: flake8-simplify rules to simplify code
  • PL: pylint rules to "check[] for errors, enforce[] a coding standard, look[] for code smells, and [] make suggestions about how the code could be refactored"
  • FURB: refurb, "for refurbishing and modernizing Python codebases"

I thought these made useful improvements and would be helpful for maintaining our code going forward.

But don't enable ruff format preview mode, since we don't want their parentheses formatting yet (hug_parens_with_braces_and_square_brackets)
These rules can be used again now that preview mode is enabled
Comment on lines +1438 to +1439
s += "X units: {:s}\n".format(self.xmode or "None")
s += "Y units: {:s}\n".format(self.ymode or "None")
Copy link
Contributor

Choose a reason for hiding this comment

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

I dislike this change---it relies on a subtlety unique to python that not even many intermediate python programmers know, just for the sake of a little brevity. Could we disable this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Your alternate idea: convert both to f"{str(self.xymode)}" for maximum conciseness.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is almost certainly FURB110

@@ -95,7 +95,7 @@ def read(filename, verbose=False, cal_kwargs=None):
energy_channel_pairs = []
energy_res_pairs = [] # currently unused
energy_eff_pairs = [] # currently unused
with Path(filename).open() as f:
with Path(filename).open(encoding="utf-8") as f:
Copy link
Contributor

Choose a reason for hiding this comment

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

is this really necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is PLW1514.

@@ -31,7 +31,7 @@ def sim_data(x_min, x_max, y_func, num_x=200, binning="linear", **params):
edges = np.linspace(x_min, x_max, num_x, dtype=float)
elif binning == "sqrt":
edges = np.linspace(np.sqrt(x_min), np.sqrt(x_max), num_x)
edges = edges**2
edges **= 2
Copy link
Contributor

Choose a reason for hiding this comment

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

this exists? TIL

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure I like it

@markbandstra
Copy link
Member Author

I will redo this PR without implementing ruff's preview mode since it includes unstable rules.

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