Skip to content

gh-144782: Make sure that ArgumentParser instances are pickleable#144783

Open
mauvilsa wants to merge 9 commits intopython:mainfrom
mauvilsa:argparse-pickleable
Open

gh-144782: Make sure that ArgumentParser instances are pickleable#144783
mauvilsa wants to merge 9 commits intopython:mainfrom
mauvilsa:argparse-pickleable

Conversation

@mauvilsa
Copy link

@mauvilsa mauvilsa commented Feb 13, 2026

Changes so that argparse.ArgumentParser instances are pickleable and a unit test so that this behavior is preserved.

Fixes issue #144782

@python-cla-bot
Copy link

python-cla-bot bot commented Feb 13, 2026

All commit authors signed the Contributor License Agreement.

CLA signed

Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
@mauvilsa mauvilsa marked this pull request as ready for review February 13, 2026 14:03
@mauvilsa
Copy link
Author

Recent changes in 3.14.3 is making jsonargparse tests fail, see actions/runs/21987177775. Would be good to backport this to 3.14, otherwise pytorch-lightning multiprocessing will be broken on 3.14.

From what I understood in the docs this pull request should have a needs backport to 3.14 label?

@mauvilsa mauvilsa requested a review from johnslavik February 13, 2026 14:32
…Y8TKj.rst

Co-authored-by: AN Long <aisk@users.noreply.github.com>
@mauvilsa mauvilsa requested a review from aisk February 13, 2026 21:37
Copy link
Member

@johnslavik johnslavik left a comment

Choose a reason for hiding this comment

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

Sorry for the review ping-pong, I've had a few new thoughts/suggestions in this round. I don't think there's anything else that will come out on my side. Nice work!

@johnslavik
Copy link
Member

johnslavik commented Feb 13, 2026

From what I understood in the docs this pull request should have a needs backport to 3.14 label?

Bug fixes are backported, yes (to 3.13 as well at the time of writing). The labels can be applied at any point, even after merge. I'm leaving the decision whether we backport this or not to @savannahostrowski -- I'm not sure if this is a bug fix and not a feature, but it seems very reasonable to backport it, so you have a +1 from me in any case!

mauvilsa and others added 2 commits February 14, 2026 07:49
Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
@mauvilsa mauvilsa requested a review from johnslavik February 14, 2026 07:12
Copy link
Member

@johnslavik johnslavik left a comment

Choose a reason for hiding this comment

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

I filed mauvilsa#1 with some final suggestions that came to my mind again. Please consider merging them, and that will be all from me :-) The changes look good to me, thanks!

Copy link
Member

@savannahostrowski savannahostrowski left a comment

Choose a reason for hiding this comment

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

I think this makes sense to fix, especially now that the workarounds don't work as a result of changes in 3.14. I think this is sufficiently low risk so I'm okay with backporting it.

However, can we frame this as bug/regression and not a new feature? To me, what we're doing here is just making it so that we aren't blocking pickling, not adding a new feature. As such, I don't think this warrants a versionchanged entry, and I'd opt to reframe the news entry accordingly.

@bedevere-app
Copy link

bedevere-app bot commented Feb 14, 2026

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.

@savannahostrowski savannahostrowski added the needs backport to 3.14 bugs and security fixes label Feb 14, 2026
@johnslavik
Copy link
Member

johnslavik commented Feb 15, 2026

@mauvilsa, I've prepared mauvilsa#2 adding the requested changes.
@savannahostrowski, do we backport to 3.13 too? It reproduces in that branch as well.

@savannahostrowski
Copy link
Member

savannahostrowski commented Feb 15, 2026

I think we should leave 3.13 as is, since that still works in jsonargparse. We just made changes to colorization in 3.14 that fundamentally break this and exacerbate the issue.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants