Skip to content

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

Merged
picnixz merged 75 commits into
python:mainfrom
furkanonder:issue-99631-2
Jul 12, 2025
Merged

gh-99631: Add custom loads and dumps support for the shelve module#118065
picnixz merged 75 commits into
python:mainfrom
furkanonder:issue-99631-2

Conversation

@furkanonder

@furkanonderfurkanonder commented Apr 18, 2024

Copy link
Copy Markdown
Contributor

Comment threadLib/shelve.py
Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst Outdated
Comment threadLib/shelve.py Outdated

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

BsdDbShelf still uses pickle.Unpickler directly to walk the keys.

Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst Outdated
Comment threadLib/shelve.py Outdated
@bedevere-app

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@@ -160,18 +181,20 @@ Restrictions
modules. The *dict* object passed to the constructor must support those
methods. This is generally accomplished by calling one of
:func:`!bsddb.hashopen`, :func:`!bsddb.btopen` or :func:`!bsddb.rnopen`. The

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

By the way, we also need to update this sentence (from bsddb to berkeleydb). bsddb is deprecated according to https://www.jcea.es/programacion/pybsddb.htm.

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.

It looks like the change would be bigger: it’s a new module (although the docs are not very clear) with maybe a new API.

Updating or deprecating this should be discussed in its own ticket 🙂

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, I agree on it. It would be better to open a new ticket to discuss this issue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... or consider instead opening topic on Discourse; this may need a larger audience than what you'd get on the bug tracker.

@furkanonderfurkanonder added awaiting review and removed 3.14 bugs and security fixes awaiting changes labels Sep 13, 2024

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

Some few nitpicks that I thought of. Some should be addressed (3.15 -> next + self.subTest(proto=5)`, while others are purely cosmetics.

I'll let a core dev check other stuff (

This core dev is now me.

Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst Outdated
Comment threadLib/test/test_shelve.py Outdated
Comment threadLib/test/test_shelve.py Outdated
Comment threadLib/test/test_shelve.py Outdated
Comment threadLib/test/test_shelve.py Outdated
Comment threadLib/test/test_shelve.py Outdated
@furkanonder
furkanonder requested a review from picnixzMay 30, 2025 16:04

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

Almost done!

Comment threadDoc/library/shelve.rst Outdated
Comment threadDoc/library/shelve.rst Outdated
Comment threadLib/test/test_shelve.py Outdated
Comment threadLib/test/test_shelve.py Outdated
@picnixz
picnixz merged commit dda70fa into python:mainJul 12, 2025
@picnixz

picnixz commented Jul 12, 2025

Copy link
Copy Markdown
Member

Oof, I forgot to merge main. Well, I don't think something for shelve changed in between and there were no conflicts, so it should be fine. Thanks for waiting!

Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
…e` module (python#118065)
Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz added a commit to picnixz/cpython that referenced this pull request Jul 13, 2025
…e` module (python#118065)
Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@furkanonder

Copy link
Copy Markdown
ContributorAuthor

Oof, I forgot to merge main. Well, I don't think something for shelve changed in between and there were no conflicts, so it should be fine. Thanks for waiting!

Thanks for the merge!

taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
…e` module (python#118065)
Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
…e` module (python#118065)
Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stdlibStandard Library Python modules in the Lib/ directory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@furkanonder@picnixz@encukou@merwok@eendebakpt@erlend-aasland