-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[refactor] Cleanup python imports #2231
Conversation
@@ -2,6 +2,7 @@ | |||
from taichi.core.settings import * | |||
from taichi.core.record import * | |||
from taichi.core.logging import * | |||
from taichi.core.primitive_types import * |
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.
Not sure how I feel about this... It's bad to do wildcard import. On the other hand, there are arguments saying that doing this in __init__.py
is an exception, in order to lift the symbols into the package level. If not written this way, we would have to write from taichi.x import float32, f32, float64, f64, ..., u64
twice, which is quite repetitive (and equally hard to maintain). What's your opinion :) ?
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 think the current solution should be good, given taichi.core.primitive_types
already clearly defined the __all__
variable, listing the names to export :-)
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.
Awesome! LGTM. It's so nice to see the imports are now much more organized than the previous version!
@@ -2,6 +2,7 @@ | |||
from taichi.core.settings import * | |||
from taichi.core.record import * | |||
from taichi.core.logging import * | |||
from taichi.core.primitive_types import * |
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 think the current solution should be good, given taichi.core.primitive_types
already clearly defined the __all__
variable, listing the names to export :-)
Related issue = #2223
[Click here for the format server]