◐ Shell
clean mode source ↗

gh-111835: Add seekable method to mmap.mmap by corona10 · Pull Request #111852 · python/cpython

@corona10

@serhiy-storchaka cc @vstinner
As the issue reporter said, they can use seekable by subclassing mmap itself.
But this change definitely helpful to people who need to use library which checks that the file object supports seekable() method.
(I don't want to say it must be added but it is good to be added)

vstinner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@serhiy-storchaka

mmap.seek() always return None. It can make mmap not appropriate substitution for files. Adding seekable() method can break existing code.

@corona10

mmap.seek() always return None. It can make mmap not appropriate substitution for files. Adding seekable() method can break existing code.

I left a comment for this.
#111835 (comment)
But I'm not sure how it will break the user's code even if we update seek() to return the current position.
I can not guess how people are doing something with None value.

serhiy-storchaka

vstinner

vstinner

vstinner

serhiy-storchaka

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

vstinner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@corona10

aisk pushed a commit to aisk/cpython that referenced this pull request

Feb 11, 2024

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request

Sep 2, 2024