Skip to content

Add seekable method for mmap.mmap #111835

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.

Sign up for GitHub

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

Jump to bottom
Closed
technillogue opened this issue Nov 8, 2023 · 4 comments
Closed

Add seekable method for mmap.mmap #111835

technillogue opened this issue Nov 8, 2023 · 4 comments
Labels
type-feature A feature request or enhancement

Comments

@technillogue
Copy link
Contributor

technillogue commented Nov 8, 2023 •
edited by bedevere-app bot
Loading

Bug report

Bug description:

mmap.mmap has a seek() method, but no seekable() method. Because of this, it cannot be used an argument for ZipFile, which requires a file-like object with seekable. A small patch is enough to fix this

class SeekableMmap(mmap.mmap):
    def seekable(self):
        return True

Now SeekableMmap can be passed to ZipFile without issue. It's easy to fix, but I can't think of any reasons why mmap shouldn't have a seekable method.

CPython versions tested on:

3.8, 3.11

Operating systems tested on:

Linux

Linked PRs
@technillogue technillogue added the type-bug An unexpected behavior, bug, or error label Nov 8, 2023
@Eclips4 Eclips4 added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Nov 8, 2023
@Eclips4 Eclips4 changed the title mmap doesn't have a seekable method Add seekable method for mmap.mmap Nov 8, 2023
corona10 added a commit to corona10/cpython that referenced this issue Nov 8, 2023
@corona10
pythongh-111835: Add seekable method to mmap.mmap
4bac18a
@bedevere-app bedevere-app bot mentioned this issue Nov 8, 2023
Merged
corona10 added a commit to corona10/cpython that referenced this issue Nov 8, 2023
@corona10
pythongh-111835: Add seekable method to mmap.mmap
d699f98
corona10 added a commit to corona10/cpython that referenced this issue Nov 8, 2023
@corona10
Merge branch 'main' into pythongh-111835
8a12113
corona10 added a commit to corona10/cpython that referenced this issue Nov 8, 2023
@corona10
Merge remote-tracking branch 'origin/pythongh-111835' into pythongh-1…
1b8d6cc 
…11835
@serhiy-storchaka
Copy link
Member

mmap.seek() returns None. The seek() method of other file-like objects return the new file position. Would not it cause issues?

technillogue added a commit to technillogue/cpython that referenced this issue Nov 8, 2023
@technillogue
pythongh-111835: Add seekable method to mmap.mmap
f38cc90
@technillogue technillogue mentioned this issue Nov 8, 2023
Closed
@technillogue
Copy link
Contributor Author

I also opened a PR #111865 for this, I just took a while between pushing the code and opening the PR.

@corona10
Copy link
Member

corona10 commented Nov 8, 2023 •
edited
Loading

mmap.seek() returns None. The seek() method of other file-like objects return the new file position. Would not it cause issues?

Yeah, in that case, we can make a seek() method to return where value. It will break the user's code actually?
In my naive idea, people cannot do something with None value.

corona10 added a commit that referenced this issue Nov 9, 2023
@corona10
gh-111835: Add seekable method to mmap.mmap (gh-111852)
6046aec
@corona10
Copy link
Member

corona10 commented Nov 9, 2023

@technillogue

#111852 is landed, this feature will be available from CPython 3.13
Thanks for the suggestion!

@serhiy-storchaka @vstinner
Thanks for your valuable feedback and reviews!

@corona10 corona10 closed this as completed Nov 9, 2023
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@corona10 @aisk
pythongh-111835: Add seekable method to mmap.mmap (pythongh-111852)
3706873
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@corona10 @Glyphack
pythongh-111835: Add seekable method to mmap.mmap (pythongh-111852)
1ffbd09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@technillogue @serhiy-storchaka @corona10 @Eclips4