Skip to content

Fix E401,E701,E711,E712,E721,F402,F403,F821 errors found by ruff #1991

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
fedelibre merged 6 commits into from
Jun 16, 2025
Merged

Fix E401,E701,E711,E712,E721,F402,F403,F821 errors found by ruff #1991

fedelibre merged 6 commits into from
Jun 16, 2025

Conversation

@fedelibre
Copy link
Member

@fedelibre fedelibre commented Jun 6, 2025

See the initial list in
#1525 (comment)

I've started working on the easier errors with the smaller number of occurences.

To complete this PR I think I'll give a deeper look into these ones in the next days:

   2	E722	[ ] bare-except
   2	F403	[ ] undefined-local-with-import-star
   1	F402	[ ] import-shadowed-by-loop-var

@fedelibre fedelibre marked this pull request as draft June 6, 2025 21:39
bmjcode
bmjcode requested changes Jun 7, 2025
View reviewed changes
bmjcode
bmjcode approved these changes Jun 10, 2025
View reviewed changes
Copy link
Collaborator

@bmjcode bmjcode 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 to me.

@fedelibre
Copy link
Member Author

fedelibre commented Jun 10, 2025

@bmjcode What do you think about this error? See also this discussion.

$ ruff check . --select F403
frescobaldi/debug.py:68:1: F403 `from PyQt6.QtCore import *` used; unable to detect undefined names
   |
67 | # be friendly and import Qt stuff
68 | from PyQt6.QtCore import *
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ F403
69 | from PyQt6.QtGui import *
   |

frescobaldi/debug.py:69:1: F403 `from PyQt6.QtGui import *` used; unable to detect undefined names
   |
67 | # be friendly and import Qt stuff
68 | from PyQt6.QtCore import *
69 | from PyQt6.QtGui import *
   | ^^^^^^^^^^^^^^^^^^^^^^^^^ F403
   |

Found 2 errors.

debug.py is not used by the regular application and can be activated only if you launch Frescobaldi from the python shell like so:

from frescobaldi.debug import *

That said, I'm just trying to clear all the errors found by ruff in order to enable the ruff check for future PRs. If there's no easy fix for this, we can tell ruff to skip the F403 errors or disable the check only for these two lines.

What do you suggest?

@fedelibre fedelibre marked this pull request as ready for review June 10, 2025 20:09
@fedelibre
Copy link
Member Author

fedelibre commented Jun 10, 2025

I've added a commit to fix this error:

$ ruff check . --select F402
frescobaldi/externalchanges/widget.py:152:27: F402 Import `document` from line 36 shadowed by loop variable
    |
150 |             diritem.setFlags(Qt.ItemFlag.ItemIsEnabled)
151 |             diritem.setIcon(0, icons.get('folder-open'))
152 |             for filename, document in sorted(dirs[dirname],
    |                           ^^^^^^^^ F402
153 |                                               key=lambda item: util.naturalsort(item[0])):
154 |                 fileitem = QTreeWidgetItem()
    |

Found 1 error.

@fedelibre
Copy link
Member Author

fedelibre commented Jun 10, 2025

I fixed one of these F821 errors:

$ ruff check . --statistics
2038	F821	[ ] undefined-name

The remaining 2038 refer to the use of _.
_ is defined here and installed in the builtins. So inside Frescobaldi _ is actually defined and this explains why it works. But ruff checks every single file without knowing this. It seems we have two options:

  1. Tell ruff to ignore _
[tool.ruff]
ignore-names = ["_"]
  1. Add a fake import on top of all files containing _
from frescobaldi.i18n import _

I would go for 1.
Consider that #1285 might solve this problem in a cleaner way.

@bmjcode
Copy link
Collaborator

bmjcode commented Jun 10, 2025

@fedelibre

On F403: I'm trying to figure out what those Qt imports are supposed to do. They come at the very end of debug.py, so nothing in that file uses them. I would guess from context they're intended for interactive use, except importing them in debug.py doesn't actually pass them through to the interactive shell:

>>> from frescobaldi.debug import *
created: <document.EditorDocument object at 0x000001D8BE5FE7B0>
>>> w = QMainWindow()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'QMainWindow' is not defined

I'd just delete the Qt imports unless someone can explain what they're there for.

On F821: I'd also go with option 1. Personally I think adding things to builtins is the wrong kind of clever, in part because it breaks tools like ruff, but I don't want to change it if it's working now. I think avoiding all those imports is probably why it was done that way in the first place.

@fedelibre
Copy link
Member Author

fedelibre commented Jun 11, 2025

Ok, I've pushed two more commits to fix those errors.

Here's the current situation:

$ ruff check . --statistics
308	F401	[ ] unused-import
 68	E741	[ ] ambiguous-variable-name
 24	E402	[ ] module-import-not-at-top-of-file
 23	E731	[ ] lambda-assignment
 20	E713	[*] not-in-test
 20	F841	[ ] unused-variable
 12	F811	[ ] redefined-while-unused
  2	E722	[ ] bare-except
Found 477 errors.
[*] 273 fixable with the `--fix` option (41 hidden fixes can be enabled with the `--unsafe-fixes` option).

This means that this PR fixed the following 8 errors:

2039	F821	[ ] undefined-name
   5	E721	[ ] type-comparison
   2	E701	[ ] multiple-statements-on-one-line-colon
   2	E712	[ ] true-false-comparison
   2	F403	[ ] undefined-local-with-import-star
   1	E401	[*] multiple-imports-on-one-line
   1	E711	[ ] none-comparison
   1	F402	[ ] import-shadowed-by-loop-var

Enough for this PR.
I'll wait a few more days before merging it, in case @jeanas finds the time to review it.

@fedelibre fedelibre changed the title Fix E401,E701,E711,E712,E721 errors found by ruff Fix E401,E701,E711,E712,E721,F402,F403,F821 errors found by ruff Jun 11, 2025
@fedelibre fedelibre merged commit c0552e1 into master Jun 16, 2025
@fedelibre fedelibre deleted the ruff-easier-errors branch June 16, 2025 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bmjcode bmjcode bmjcode approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fedelibre @bmjcode