Skip to content

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

gh-99631: Add custom loads and dumps support for the shelve module#99632
furkanonder wants to merge 17 commits into
python:mainfrom
furkanonder:issue-99631

Conversation

@furkanonder

@furkanonderfurkanonder commented Nov 20, 2022

Copy link
Copy Markdown
Contributor

@merwokmerwok 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

Copy link
Copy Markdown
Member

Could you add docs and tests?

@serhiy-storchaka

Copy link
Copy Markdown
Member

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

@isidentical

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)

@furkanonderfurkanonder changed the title gh-99631: Add custom unpickler and pickler support for the shelve modulegh-99631: Add custom loads and dumps support for the shelve moduleDec 3, 2022
@furkanonder

Copy link
Copy Markdown
ContributorAuthor

Doc and test have been added. @merwok

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

@isidenticalisidentical 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.

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

Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst
Comment threadLib/test/test_shelve.py Outdated
@netlify

netlifyBot commented Dec 7, 2022

Copy link
Copy Markdown

Deploy Preview for python-cpython-preview canceled.

NameLink
🔨 Latest commit11cbfbf
🔍 Latest deploy loghttps://app.netlify.com/sites/python-cpython-preview/deploys/63934b91a06334000855fb77

@serhiy-storchaka

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?

@furkanonder

Copy link
Copy Markdown
ContributorAuthor

What happens if only serializer or only deserializer are passed?

elif (serializerisNoneanddeserializerisnotNone) or (deserializerisNoneandserializerisnotNone):
raiseShelveError("Serializer and deserializer must be defined together.")

In this case, shelve raises an error.

Comment threadLib/shelve.py Outdated
Comment threadLib/shelve.py Outdated
@furkanonder
furkanonder requested review from isidentical and rhettinger and removed request for isidentical and rhettingerDecember 20, 2022 20:05
@furkanonderfurkanonder closed this by deleting the head repository Dec 23, 2022
@furkanonder
furkanonder requested review from isidentical and rhettinger and removed request for isidentical and rhettingerDecember 23, 2022 18:52
Comment threadLib/shelve.py
@encukou

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

@furkanonder

Copy link
Copy Markdown
ContributorAuthor

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.12only security fixesawaiting mergestdlibStandard Library Python modules in the Lib/ directorytype-featureA feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@furkanonder@merwok@serhiy-storchaka@isidentical@encukou@rhettinger@bedevere-bot