Skip to content

[flake8-import-conventions] Add import numpy.typing as npt to default flake8-import-conventions.aliases #17133

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

Merged
merged 2 commits into from
Apr 2, 2025

Conversation

amin-not-found
Copy link
Contributor

Summary

Adds import numpy.typing as npt to default in flake8-import-conventions.aliases
Resolves #17028

Test Plan

Manually ran local ruff on the altered fixture and also ran cargo test

Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good except for one question about the conventional test case.

If you end up changing that, it might be nice to move the new tests to the bottom of the test file too. That will make it much easier to review the changes since it won't move any of the other snapshots!

@MichaReiser does this need to be in preview? I think it's a new default restriction on the allowed name for the numpy.typing import, but I guess we'll see if there are any ecosystem hits.

@@ -35,6 +37,7 @@
import dask.dataframe as dd # conventional
import matplotlib.pyplot as plt # conventional
import numpy as np # conventional
import numpy as npt # conventional
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this one supposed to be import numpy.typing as npt?

@ntBre ntBre added the configuration Related to settings and configuration label Apr 1, 2025
Copy link
Contributor

github-actions bot commented Apr 1, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@amin-not-found
Copy link
Contributor Author

Is this one supposed to be import numpy.typing as npt?

Yes, that's right. I forgot to type typing there. It should be fixed now.

it might be nice to move the new tests to the bottom of the test file too

I have changed it, but I was trying to keep the structure. Maybe I can reorder the imports by library instead, so that it has some kind of structure that is easy to keep whit new additions? Something like this:

import altair  # unconventional
import altair as altr  # unconventional
import altair as alt  # conventional

import dask.array  # unconventional
import dask.array as darray  # unconventional
...

Or maybe not if that's too much change.

@ntBre
Copy link
Contributor

ntBre commented Apr 1, 2025

Nice! Yeah it's always a tradeoff on keeping the structure or not. I think it's okay like you have it now though, without worrying about rearranging the other cases.

Let's wait for confirmation on whether this needs to be preview-gated or not, but otherwise this looks good to me.

@amin-not-found
Copy link
Contributor Author

Yeah I get it and also appreciate your feedback

@MichaReiser
Copy link
Member

I think it's fine to ship this as is. I suspect that most numpy users added npt to their convention configuration already and, if not, it only removes the need for some suppression comments.

@MichaReiser MichaReiser merged commit e1b5b0d into astral-sh:main Apr 2, 2025
22 checks passed
maxmynter pushed a commit to maxmynter/ruff that referenced this pull request Apr 3, 2025
…lt `flake8-import-conventions.aliases` (astral-sh#17133)

## Summary
Adds import `numpy.typing as npt` to `default in
flake8-import-conventions.aliases`
Resolves astral-sh#17028

## Test Plan
Manually ran local ruff on the altered fixture and also ran `cargo test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unconventional-import-alias (ICN001): Add import numpy.typing as npt to default lint.flake8-import-conventions.aliases
3 participants