Add support for pathlib.Path instances #123
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`pathlib.Path` instances are commonly used, but the `openslide.OpenSlide` class was not compatible with the `pathlib.Path` type. This commit converts the `slide._filename` attribute to a `str` type in `__init__`. This means that the `openslide.OpenSlide` class can now accept a `pathlib.Path` instance.
jaharkes
reviewed
Jun 3, 2021
|
Yes, it looks like we do not need to change self._filename, and only have to use str(filename) in the calls to lowlevel.open and low-level.format.
The test change is straightforward because we already dropped support for python < 3.6 and so we can assume pathlib.Path is always available and always pass Path instances.
On June 3, 2021 11:49:43 AM EDT, Jakub Kaczmarzyk ***@***.***> wrote:
***@***.*** commented on this pull request.
…
> + self._filename = str(filename)
self._osr = lowlevel.open(filename)
Thanks for the comment. What if we also converted the input to
`lowlevel.detect_format()` to a `str`? I agree that there should be a
test using `pathlib.Path` instances.
|
|
Thank you for making those changes @jaharkes. This looks good to go on my end. |
bgilbert
added a commit
to bgilbert/openslide-python
that referenced
this pull request
Oct 20, 2024
- Starting in 1.2.0, OpenSlide() and OpenSlide.detect_format() have failed to accept filename arguments formatted as pre-encoded bytes because str(b'abc') == "b'abc'". In addition, filename arguments with invalid types (such as None) have been stringified and passed to OpenSlide, rather than raising an exception during conversion; we even had tests for this (!). - lowlevel has always encoded filename arguments to UTF-8, but on non-Windows it should have used the Python filesystem encoding instead (usually UTF-8 but not always). On Windows, OpenSlide 4.0.0+ expects UTF-8 rather than arbitrary bytes. (OpenSlide < 4.0.0 expects the system codepage, which isn't very useful in practice because of its limited character set, so we ignore that case for now.) - Type hints did not allow filename arguments to be bytes, nor did they allow os.PathLike subclasses which were not pathlib.Path (such as pathlib.PurePath). Accept str, bytes, or os.PathLike for all filename arguments, and properly convert them to bytes for OpenSlide. Fixes: 98c11bd ("Add support for pathlib.Path instances (openslide#123)") Fixes: 5644229 ("tests: test passing invalid types to OpenSlide constructor")
bgilbert
added a commit
to bgilbert/openslide-python
that referenced
this pull request
Oct 20, 2024
- Starting in 1.2.0, OpenSlide() and OpenSlide.detect_format() have failed to accept filename arguments formatted as bytes because str(b'abc') == "b'abc'". In addition, filename arguments with invalid types (such as None) have been stringified and passed to OpenSlide, rather than raising an exception during conversion; we even had tests for this (!). - lowlevel has always encoded filename arguments to UTF-8, but on non-Windows it should have used the Python filesystem encoding instead (usually UTF-8 but not always). On Windows, OpenSlide 4.0.0+ expects UTF-8 rather than arbitrary bytes. (OpenSlide < 4.0.0 expects the system codepage, which isn't very useful in practice because of its limited character set, so we ignore that case for now.) - Type hints did not allow filename arguments to be bytes, nor did they allow os.PathLike subclasses which were not pathlib.Path (such as pathlib.PurePath). Accept str, bytes, or os.PathLike for all filename arguments, and properly convert them to bytes for OpenSlide. Fixes: 98c11bd ("Add support for pathlib.Path instances (openslide#123)") Fixes: 5644229 ("tests: test passing invalid types to OpenSlide constructor")
bgilbert
added a commit
to bgilbert/openslide-python
that referenced
this pull request
Oct 20, 2024
- Starting in 1.2.0, OpenSlide() and OpenSlide.detect_format() have failed to accept filename arguments formatted as bytes because str(b'abc') == "b'abc'". In addition, filename arguments with invalid types (such as None) have been stringified and passed to OpenSlide, rather than raising an exception during conversion; we even had tests for this (!). - lowlevel has always encoded filename arguments to UTF-8, but on non-Windows it should have used the Python filesystem encoding instead (usually UTF-8 but not always). On Windows, OpenSlide 4.0.0+ expects UTF-8 rather than arbitrary bytes. (OpenSlide < 4.0.0 expects the system codepage, which isn't very useful in practice because of its limited character set, so we ignore that case for now.) - Type hints did not allow filename arguments to be bytes, nor did they allow os.PathLike subclasses which were not pathlib.Path (such as pathlib.PurePath). Accept str, bytes, or os.PathLike for all filename arguments, and properly convert them to bytes for OpenSlide. Fixes: 98c11bd ("Add support for pathlib.Path instances (openslide#123)") Fixes: 5644229 ("tests: test passing invalid types to OpenSlide constructor") Signed-off-by: Benjamin Gilbert <[email protected]>
bgilbert
added a commit
to bgilbert/openslide-python
that referenced
this pull request
Oct 20, 2024
- Starting in 1.2.0, OpenSlide() and OpenSlide.detect_format() have failed to accept filename arguments formatted as bytes because str(b'abc') == "b'abc'". In addition, filename arguments with invalid types (such as None) have been stringified and passed to OpenSlide, rather than raising an exception during conversion; we even had tests for this (!). - lowlevel has always encoded filename arguments to UTF-8, but on non-Windows it should have used the Python filesystem encoding instead (usually UTF-8 but not always). On Windows, OpenSlide 4.0.0+ expects UTF-8 rather than arbitrary bytes. (OpenSlide < 4.0.0 expects the system codepage, which isn't very useful in practice because of its limited character set, so we ignore that case for now.) - Type hints did not allow filename arguments to be bytes, nor did they allow os.PathLike subclasses which were not pathlib.Path (such as pathlib.PurePath). Accept str, bytes, or os.PathLike for all filename arguments, and properly convert them to bytes for OpenSlide. Fixes: 98c11bd ("Add support for pathlib.Path instances (openslide#123)") Fixes: 5644229 ("tests: test passing invalid types to OpenSlide constructor") Signed-off-by: Benjamin Gilbert <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pathlib.Pathinstances are commonly used, but theopenslide.OpenSlideclass was not compatible with thepathlib.Pathtype. This commit converts theslide._filenameattribute to astrtype in__init__. This means that theopenslide.OpenSlideclass can now accept apathlib.Pathinstance.