-
-
Notifications
You must be signed in to change notification settings - Fork 939
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
Ensure absolute imports #2789
base: main
Are you sure you want to change the base?
Ensure absolute imports #2789
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth to double check that the internal imports are really absolutely pointing to the deepest submodule possible. We can trim after once we make sure and control over the circular dependencies
kornia/augmentation/__init__.py
Outdated
from kornia.augmentation._2d.base import AugmentationBase2D, RigidAffineAugmentationBase2D | ||
from kornia.augmentation import auto | ||
from kornia.augmentation import container | ||
from kornia.augmentation._2d import CenterCrop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe here makes sense wildcards and inside the sub module we add what to be imported by the all variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't understand what do you mean
kornia/feature/__init__.py
Outdated
from .sold2 import SOLD2, SOLD2_detector | ||
from .sosnet import SOSNet | ||
from .tfeat import TFeat | ||
from kornia.feature.affine_shape import LAFAffineShapeEstimator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe same here with wildcards
16d4321
to
9f1001e
Compare
We should manually review each one for it, or do you know how to automate it? |
@@ -5,12 +5,11 @@ | |||
import torch | |||
|
|||
from kornia.core import stack | |||
from kornia.geometry.calibration.distort import distort_points, tilt_projection | |||
from kornia.geometry.linalg import transform_points | |||
from kornia.geometry.transform import remap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnnv1 just as random example, this import is not full absolute to where it’s actually implemented. I’m pretty sure there way more in code base
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, needs to review/check each one
This patch bans relative imports, and moves all our relative imports to absolute imports using ruff
based on https://docs.astral.sh/ruff/settings/#lint_flake8-tidy-imports_ban-relative-imports
To reproduce:
tool.ruff.lint.flake8-tidy-imports
rule to ban relative importsruff run ./ --fix --unsafe-fixes
To be merged after #2788