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

Start applying ruff rules #269

Merged
merged 38 commits into from
Feb 3, 2025
Merged

Start applying ruff rules #269

merged 38 commits into from
Feb 3, 2025

Conversation

DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Jan 18, 2025

@skjerns Ready for review.

F401 imported but unused
F841 Local variable is assigned to but never used
E402 Module level import not at top of file
E701 Multiple statements on one line (colon)
E713 Test for membership should be `not in`
E722 Do not use bare `except`
E731 Do not assign a `lambda` expression, use a `def`
W293 Blank line contains whitespace
W391 Extra newline at end of file
UP006 Use `dist` instead of `Dist` for type annotation
UP006 Use `list` instead of `List` for type annotation
UP007 Use `X | Y` for type annotations
UP009 UTF-8 encoding declaration is unnecessary
UP012 Unnecessary call to `encode` as UTF-8
UP020 Use builtin `open`
f.writeSamples(data_list)
f.close()

f = pyedflib.EdfReader(self.bdfplus_data_file)
data1_read = f.readSignal(0)
data2_read = f.readSignal(1)
f._close
f._close()
Copy link
Collaborator

Choose a reason for hiding this comment

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

uh strange that this never gave any access/locking problems

C408 Unnecessary `dict()` call (rewrite as a literal)
C419 Unnecessary list comprehension
ISC001 Implicitly concatenated string literals on one line
ISC002 Implicitly concatenated string literals over multiple lines
ISC003 Explicitly concatenated string should be implicitly concatenated
SIM102 Use a single `if` statement instead of nested `if` statements
SIM201 Use `... != ...` instead of `not ... == ...`
FLY002 Consider f-string instead of string join
PERF401 Use a list comprehension to create a transformed list
FURB110 Replace ternary `if` expression with `or` operator
FURB113 Use `extend(()` instead of repeatedly calling `append()`
FURB167 Use of regular expression alias
FURB171 Membership test against single-item container
FURB188 Prefer `str.removeprefix()` over conditionally replacing with slice.
RUF022 `__all__` is not sorted
RUF027 Possible f-string without an `f` prefix
RUF036 `None` not at the end of the type annotation.
RUF039 First argument to `re.match()` is not raw string
RUF039 First argument to `re.search()` is not raw string
RUF039 First argument to `re.compile()` is not raw string
@skjerns
Copy link
Collaborator

skjerns commented Jan 27, 2025

thanks! looking all good :)

is there a necessity to use add a github action for ruff? or any other automated checking tool?

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Jan 27, 2025

Two options:

  • use pytest-ruff within the existing test GitHub Action (that's what I've chosen in 88ec4c1)
  • create a new ruff job

@skjerns
Copy link
Collaborator

skjerns commented Jan 27, 2025

Two options:

  • use pytest-ruff within the existing test GitHub Action (that's what I've chosen in 88ec4c1)
  • create a new ruff job

I think it would be nicer to have one dedicated job, that way it is easier to see if errors are caused by cosmetic changes or functional tests failing.

Also using the current matrix layout the pytest --ruff will run redundantly several times needlessly.

Replace flake8 with ruff.
* Document that Python 3.8 is still tested
* Document that Python 3.13 is supported
* Update GitHub actions
@DimitriPapadopoulos
Copy link
Contributor Author

Updated to run ruff directly from a GitHub Action.

@skjerns
Copy link
Collaborator

skjerns commented Feb 3, 2025

thanks! looking good :)

@skjerns skjerns merged commit 8b3c29f into holgern:master Feb 3, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants