Skip to content

ruff fixes #196

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

Jump to bottom
Merged
merged 45 commits into from
Sep 2, 2024
Merged

ruff fixes #196

merged 45 commits into from
Sep 2, 2024

Conversation

DanielYang59
Copy link
Collaborator

@DanielYang59 DanielYang59 commented Sep 1, 2024 •
edited
Loading

ruff fixes

  • Fix T201, replace print with others if possible
  • Globally ignore SLF001, access private class member

TODOs:

  • Fix PT017 in test_io
  • Fix NPY201 in uncertainty (this might be bug, if user use NumPy 2)
  • Fix PT011, exception capture without match
  • "PT006", # pytest-parametrize-names-wrong-type
  • "COM812", # trailing comma missing
  • remove no_type_check
  • [need confirm] CI test reduce items shown in slowest durations to 20

@DanielYang59 DanielYang59 self-assigned this Sep 1, 2024
@DanielYang59 DanielYang59 added the housekeeping Keep the code base tidy. label Sep 1, 2024
@DanielYang59 DanielYang59 added the types Type all the things label Sep 1, 2024
@DanielYang59 DanielYang59 requested a review from janosh September 2, 2024 03:43
janosh
janosh requested changes Sep 2, 2024
View reviewed changes
Copy link
Owner

@janosh janosh left a comment

Choose a reason for hiding this comment

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

looks good, thank you! 👍 just a few nitpicks

tests/test_io.py
from unittest.mock import patch
from xml.etree import ElementTree # noqa: ICN001
from xml.etree import ElementTree as ET
Copy link
Owner

Choose a reason for hiding this comment

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

not a fan of this convention, i prefer to import ElementTree as is without renaming but not a strong preference either way

Copy link
Collaborator Author

@DanielYang59 DanielYang59 Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

ET is indeed less readable than ElementTree, but the official Python API doc seems to prefer ET:

This is a short tutorial for using xml.etree.ElementTree (ET in short).

And in code demo as well:

import xml.etree.ElementTree as ET
tree = ET.parse('country_data.xml')
root = tree.getroot()

Perhaps just follow the convention? Would let you decide :)

@janosh janosh merged commit 55a722d into main Sep 2, 2024
18 of 19 checks passed
@janosh janosh deleted the ruff-pmv branch September 2, 2024 07:02
@DanielYang59
Copy link
Collaborator Author

Thanks!

janosh added a commit that referenced this pull request Mar 28, 2025
@DanielYang59 @janosh
ruff fixes (#196)
368bf1d 
* ruff fixes

* fix s101 use of assert

* globally ignore SLF001, access private class member

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add todo tag

* put docstring and global ignore on top

* fix py017, properly capture exception

* Fix PT011, exception capture without match

* add TODO tag for #195 (comment)

* fix NPY201, np trapz

* use warn over print to fix T201

* add comments for globally ignored ruff rules

* guard type check only import when possible

* remove ISC001 from ignore

* add todo tag

* reduce duration items shown to 20

* import Self from typing extension

* remove global ignore of S311 as there're only 2 violation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* replace random with numpy, which also fixes S311

* fix "PT006",   # pytest-parametrize-names-wrong-type

* guard type check only import

* fix "COM812",  # trailing comma missing

* remove no type check mark

* Revert "fix "COM812",  # trailing comma missing"

This reverts commit 5ab3390.

* fix unit test of io

* NEED confirm: change width from 457 to 458

* clean up branch condition

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* use native array size arg

* revert accidental elem -> element rename

* fix random array generation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix random array generate in asset maker

* revert test_io value to 457, reason unknown

* skip eslint for now #197

* format pre-commit config

* remove eslint skip mark in CI

* fix indentation in pyproject.toml

* drop quotes in test.yml string

* fix typo in filename

* fix shaded_ys type

* remove overwriting pytest duration

* pyproject whitespace

---------

Co-authored-by: Janosh Riebesell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janosh janosh janosh approved these changes

Assignees

@DanielYang59 DanielYang59

Labels
housekeeping Keep the code base tidy. types Type all the things
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DanielYang59 @janosh