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

isort: Don't infer namespace packages as first-party (only subpackages of namespace packages) #12987

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Aug 19, 2024

Fixes #12984 (will add docs/PR description/tests after I've seen the ecosystem report)

@AlexWaygood AlexWaygood force-pushed the alex/isort-namespace-pkgs branch from 936610d to 00b20ac Compare August 19, 2024 12:22
Copy link
Contributor

github-actions bot commented Aug 19, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+18 -9 violations, +0 -0 fixes in 6 projects; 48 projects unchanged)

Snowflake-Labs/snowcli (+12 -7 violations, +0 -0 fixes)

+ scripts/cleanup.py:14:1: I001 [*] Import block is un-sorted or un-formatted
- src/snowflake/cli/_plugins/connection/util.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/git/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/nativeapp/project_model.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/snowpark/common.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/snowpark/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/spcs/compute_pool/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/spcs/image_repository/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/stage/diff.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/streamlit/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/api/cli_global_context.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/api/sql_execution.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ test_external_plugins/snowpark_hello_single_command/src/snowflakecli/test_plugins/snowpark_hello/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/notebook/test_notebooks.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/spcs/testing_utils/compute_pool_utils.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/spcs/testing_utils/spcs_services_utils.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/test_config.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/test_stage.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/testing_utils/sql_utils.py:15:1: I001 [*] Import block is un-sorted or un-formatted

apache/airflow (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ dev/perf/dags/perf_dag_1.py:22:1: I001 [*] Import block is un-sorted or un-formatted

freedomofpress/securedrop (+1 -0 violations, +0 -0 fixes)

+ journalist_gui/test_gui.py:1:1: I001 [*] Import block is un-sorted or un-formatted

latchbio/latch (+2 -2 violations, +0 -0 fixes)

+ latch_cli/services/init/example_conda/__init__.py:10:35: F401 `latch.resources.tasks.small_task` imported but unused
- latch_cli/services/init/example_conda/__init__.py:10:35: F401 `latch.resources.tasks.small_task` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
+ latch_cli/services/init/example_r/__init__.py:10:35: F401 `latch.resources.tasks.small_task` imported but unused
- latch_cli/services/init/example_r/__init__.py:10:35: F401 `latch.resources.tasks.small_task` imported but unused; consider removing, adding to `__all__`, or using a redundant alias

mlflow/mlflow (+1 -0 violations, +0 -0 fixes)

+ tests/projects/utils.py:50:1: I001 [*] Import block is un-sorted or un-formatted

pytest-dev/pytest (+1 -0 violations, +0 -0 fixes)

+ testing/_py/test_local.py:2:1: I001 [*] Import block is un-sorted or un-formatted

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
I001 23 16 7 0 0
F401 4 2 2 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+18 -9 violations, +0 -0 fixes in 6 projects; 48 projects unchanged)

Snowflake-Labs/snowcli (+12 -7 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ scripts/cleanup.py:14:1: I001 [*] Import block is un-sorted or un-formatted
- src/snowflake/cli/_plugins/connection/util.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/git/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/nativeapp/project_model.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/snowpark/common.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/snowpark/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/spcs/compute_pool/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/spcs/image_repository/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/stage/diff.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/_plugins/streamlit/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/api/cli_global_context.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ src/snowflake/cli/api/sql_execution.py:15:1: I001 [*] Import block is un-sorted or un-formatted
+ test_external_plugins/snowpark_hello_single_command/src/snowflakecli/test_plugins/snowpark_hello/manager.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/notebook/test_notebooks.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/spcs/testing_utils/compute_pool_utils.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/spcs/testing_utils/spcs_services_utils.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/test_config.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/test_stage.py:15:1: I001 [*] Import block is un-sorted or un-formatted
- tests_integration/testing_utils/sql_utils.py:15:1: I001 [*] Import block is un-sorted or un-formatted

apache/airflow (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ dev/perf/dags/perf_dag_1.py:22:1: I001 [*] Import block is un-sorted or un-formatted

freedomofpress/securedrop (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ journalist_gui/test_gui.py:1:1: I001 [*] Import block is un-sorted or un-formatted

latchbio/latch (+2 -2 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- latch_cli/services/init/example_conda/__init__.py:10:35: F401 [*] `latch.resources.tasks.small_task` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
+ latch_cli/services/init/example_conda/__init__.py:10:35: F401 `latch.resources.tasks.small_task` imported but unused
- latch_cli/services/init/example_r/__init__.py:10:35: F401 [*] `latch.resources.tasks.small_task` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
+ latch_cli/services/init/example_r/__init__.py:10:35: F401 `latch.resources.tasks.small_task` imported but unused

mlflow/mlflow (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ tests/projects/utils.py:50:1: I001 [*] Import block is un-sorted or un-formatted

pytest-dev/pytest (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ testing/_py/test_local.py:2:1: I001 [*] Import block is un-sorted or un-formatted

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
I001 23 16 7 0 0
F401 4 2 2 0 0

@AlexWaygood AlexWaygood force-pushed the alex/isort-namespace-pkgs branch 2 times, most recently from 9dd7068 to 04aecf3 Compare August 19, 2024 17:40
@AlexWaygood AlexWaygood changed the base branch from main to alex/cleanup-isort August 19, 2024 17:40
@AlexWaygood AlexWaygood force-pushed the alex/isort-namespace-pkgs branch from 04aecf3 to 42b9882 Compare August 19, 2024 17:41
Base automatically changed from alex/cleanup-isort to main August 19, 2024 18:06
An error occurred while trying to automatically change base from alex/cleanup-isort to main August 19, 2024 18:06
@AlexWaygood AlexWaygood force-pushed the alex/isort-namespace-pkgs branch from 42b9882 to aa79994 Compare August 20, 2024 09:33
Copy link

codspeed-hq bot commented Aug 20, 2024

CodSpeed Performance Report

Merging #12987 will degrade performances by 7.42%

Comparing alex/isort-namespace-pkgs (352edbb) with main (0bd258a)

Summary

❌ 1 regressions
✅ 31 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main alex/isort-namespace-pkgs Change
linter/all-rules[numpy/globals.py] 726.2 µs 784.4 µs -7.42%

@AlexWaygood AlexWaygood force-pushed the alex/isort-namespace-pkgs branch from aa79994 to 8622df9 Compare August 20, 2024 09:58
@smartYSC
Copy link

smartYSC commented Nov 6, 2024

Hey, thank you for preparing this PR! I built and tested it locally and it solves the problem I discussed in #12038. Any chance to get this merged?

@AlexWaygood
Copy link
Member Author

I'll try to find some time to work on this soon and hopefully fix the perf regression!

@smartYSC
Copy link

smartYSC commented Jan 3, 2025

Hi Alex, do you have any update on this?

Unfortunately I am not a Rust developer, otherwise I would be glad to help out!

@smartYSC
Copy link

Gentle reminder. This issue is blocking us from using ruff in our company :-(

@ollie-bell
Copy link

Gentle reminder. This issue is blocking us from using ruff in our company :-(

can you not use [tool.ruff.lint.isort.known-third-party]?

@smartYSC
Copy link

smartYSC commented Feb 26, 2025

can you not use [tool.ruff.lint.isort.known-third-party]?

As discussed in #12038 this sadly does not give the right output.

@smartYSC
Copy link

@ollie-bell to elaborate this a bit further: ruff fails to detect the namespace package correctly (no matter what config options you throw at it).

If I have a namespace package foo and the repo I lint only contains foo.bar it will never sort foo.baz correctly. It only looks at the first part before the . (foo) and decides too early what type of package it is (ignoring the fact, that bar can be found locally and baz cannot).

@strubbly
Copy link

Jut to add that we are in the same boat. We are trying to move to ruff but this issue prevents it since we can't get it to do the same as isort has always done for us. So we can't use it - it would reformat all our files.

@ntBre
Copy link
Contributor

ntBre commented Feb 27, 2025

Thanks for the updates everyone! We've been talking about this internally and are planning to have someone look into this again soon. I'm hoping to have a look this week if @dylwil3 doesn't beat me to it!

@dylwil3 dylwil3 self-assigned this Mar 1, 2025
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.

Third-party namespace packages can be incorrectly considered first-party packages by isort rules
6 participants