-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Update UnidentifiedImageError and __version__ imports #7644
Conversation
src/PIL/Image.py
Outdated
_plugins, | ||
) | ||
from ._binary import i32le, o32be, o32le | ||
from ._util import DeferredError, is_path | ||
from ._version import __version__ |
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.
Does _version
exist solely to simplify version parsing in setup.py
? In my opinion, the actual module where the version should be defined is PIL
.
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.
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.
Let's keep the __version__
import from PIL
, as that's the more "official" location (unless we can demonstrate significant performance boost, which I think is unlikely).
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.
Ok, I've pushed a commit so that __version__
is imported from the base instead.
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've created #7874 as a follow up to this, moving the definition of __version__
to __init__.py
.
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.
The reason we did this was to be able to import the version without a buildable PIL, so that it could be added to the C build automatically to facilitate the version checks there.
_version.py
is a private implementation detail, everyone but setup.py should be referencing the published PIL.__version__
or whatever we're calling it now.
Thanks! |
There are a few instances where
UnidentifiedImageError
is imported fromImage
rather than.
, and__version__
is imported from.
rather than_version
.It would seem better to import items from where they are located, instead of relying on them being imported in another place.