FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-99631: Add custom loads and dumps support for the shelve module by furkanonder · Pull Request #99632 · python/cpython · GitHub

Repository navigation

gh-99631: Add custom loads and dumps support for the shelve module - #99632

Closed
furkanonder wants to merge 17 commits into
python:mainfrom
furkanonder:issue-99631
Closed

furkanonder wants to merge 17 commits into
python:mainfrom
furkanonder:issue-99631

Conversation

furkanonder commented Nov 20, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

merwok added type-feature A feature request or enhancement stdlib Standard Library Python modules in the Lib/ directory 3.12 only security fixes labels Dec 1, 2022

merwok commented Dec 1, 2022

Copy link
Copy Markdown
Member

Could you add docs and tests?

Copy link
Copy Markdown
Member

Do we need to specify the pickler and unpickler classes or just dumps() and loads() functions?

Copy link
Copy Markdown
Member

I'd also go fordumps and loads instead of pickler/unpickler classes. With a proper definition (e.g. takes a Python object and returns bytes) I think that should make it much more simpler (it is also similar how other pickle-related APIs work outside of the stdlib)

furkanonder changed the title gh-99631: Add custom unpickler and pickler support for the shelve module gh-99631: Add custom loads and dumps support for the shelve module Dec 3, 2022

Copy link
Copy Markdown
Contributor Author

Doc and test have been added. @merwok

The pickler and unpickler classes have been replaced with dumps() and load() functions. @isidentical @serhiy-storchaka

isidentical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks a lot for your efforts @furkanonder, I have a few minor suggestions but this already looks great!

Comment thread Doc/library/shelve.rst Outdated
Comment thread Doc/library/shelve.rst
Comment thread Lib/test/test_shelve.py Outdated

netlify Bot commented Dec 7, 2022 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for python-cpython-preview canceled.

Name Link
🔨 Latest commit 11cbfbf
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/63934b91a06334000855fb77

Copy link
Copy Markdown
Member

Should we require the serializer to accept the pickle protocol? It does not make sense for other than pickle formats.

What happens if only serializer or only deserializer are passed?

Copy link
Copy Markdown
Contributor Author

What happens if only serializer or only deserializer are passed?

elif (serializer is None and deserializer is not None) or (deserializer is None and serializer is not None):
    raise ShelveError("Serializer and deserializer must be defined together.")

In this case, shelve raises an error.

Comment thread Lib/shelve.py Outdated
Comment thread Lib/shelve.py Outdated
furkanonder requested review from isidentical and rhettinger and removed request for isidentical and rhettinger December 20, 2022 20:05
furkanonder closed this by deleting the head repository Dec 23, 2022
furkanonder reopened this Dec 23, 2022
furkanonder requested review from isidentical and rhettinger and removed request for isidentical and rhettinger December 23, 2022 18:52
Comment thread Lib/shelve.py

encukou commented Apr 9, 2024

Copy link
Copy Markdown
Member

This seems to have been forgotten over end-of-year break in 2022. Do you want to revive it?

The BsdDbShelf class needs to be updated as well; its extra functions should use deserializer rather than Unpickler

Copy link
Copy Markdown
Contributor Author

I accidentally deleted my forked repository, so I can't update my old PRs. So I opened a new PR.

This file contains hidden or 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.12 only security fixes awaiting merge stdlib Standard Library Python modules in the Lib/ directory type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL