-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update linter configuration, update code base #23
Update linter configuration, update code base #23
Conversation
ioannis-vm
commented
Dec 22, 2024
- Transitions to Ruff and codespell.
- Lints and formats the code, making safe fixes and ignoring more involved existing warnings.
- Adds run_checks.sh to run checks going forward.
`ruff format`
All changes in this commit are the result of `ruff check --fix`
- variable name `id` shadows built-in `id`. - Renamed variable.
Unnecessary `dict` call (rewrite as a literal)
- too complex methods. - simply silenced warning for existing code.
- Missing docstring. - Silecned warning for existing code.
One-line docstrings should fit in one line.
"1 blank line required between summary line and description" This helps auto-generated documentation have a concise description and optionally an extended description. The concise description renders well in tables listing available methods.
"First line should end with a period"
"First line of docstring should be in imperative mood"
"Exception must not use an f-string literal, assign to variable first"
"Consider `f'{occupancy}.{source}.{data_type}'` instead of string join"
- "Function name `...` should be lowercase" - Silenced warnings for existing code.
- Argument name `DSH` should be lowercase. - Silenced warning for existing code.
- Variable `...` in function should be lowercase. - Silenced warning for existing code.
- `inplace=True` should be avoided; it has inconsistent behavior. - Silenced warning for existing code.
- Use `.to_numpy()` instead of `.values`. - Changes implemented.
- Avoid using the generic variable name `df` for DataFrames. - Ignored warning for existing code.
- "Use a list comprehension to create a transformed list." - Change implemented.
- "Use specific rule codes when using `noqa`". - `# noqa` without rule codes removed.
Issues: - Too many branches. - Too many statements. - Magic value used in comparison. All were just silenced for the existing code.
- Loop variables being modified. These operations are prone to bugs. - Silenced warnings for existing code.
- Recommended to use pathlib. - Silenced warnings for existing code.
- Missing explicit `return` at the end of function able to return non-`None` value. - Silenced warnings for existing code.
- Unnecessary assignment to `MSE` before `return` statement. - Change implemented.
Consider ``` [subDSH[:5], *subDSH[6:-1].split(',')] ``` instead of list concatenation. Change applied.
- "Use `key in dict` instead of `key in dict.keys()`" - Changes implemented.
- "`print` found" - Silenced warnings for existing code. - We might want to use the built-in logging module here in the future.
- "Missing author in TODO." - Assigned to myself.
"Missing colon in TODO"
- "Missing issue description." - Reformatted description.
Note: For some reason it shows some old commits before I force-pushed over, and opening the PR from scratch didn't remove them. Please consider the second set of commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this, John. I appreciate that you painstakingly went through all those edits and brought the codebase up to a baseline that we can start improving from.