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

chore: bump ruff version from 0.3.3 to 0.9.2 and fix some invalid/dead noqas #3282

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bluenote10
Copy link
Contributor

@bluenote10 bluenote10 commented Jan 18, 2025

I noticed that the ruff version was a bit outdated, and that there where quite a lot of spurious/dead # noqa lingering around.

This PR bumps ruff to the latest version, which has lead to a minor re-formatting of some code (mainly around fusing string literals).

I've also enabled a few ruff rules that have helped me to identify dead or malformed (# noqa <code> instead of the needed # noqa: <code> patter) noqa rules.

Regarding F401: Here we should stick to the library interface conventions from the official specs. Otherwise type checkers will not understand the symbols exported in these modules, which is a frequent paint point on user side, when libraries suppress F401. There are basically two options:

  • Maintaining an __all__ list manually.
  • Using the X as X pattern to mark these symbols as explicitly public (and to differentiate them from actual unused imports).

I felt that the latter fits better to how Aim maintains its library interface.

My plan is to fully fix all F401 in a follow-up, because the broken library interface is biting us.

@@ -41,7 +41,7 @@ async def search_tags_by_name_api(q: Optional[str] = '', factory=Depends(object_
'id': tag.uuid,
'name': tag.name,
'color': tag.color,
'description' 'run_count': len(tag.runs),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks very much like an existing bug, no?

@bluenote10
Copy link
Contributor Author

Would it be possible to move forward with this? Is there anything I can do to help speeding things up / simplifying the review?

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.

1 participant