Skip to content

[python] fix compatibility with Python 3.14 #464

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

Open
ZhongRuoyu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[python] fix compatibility with Python 3.14 #464

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions python/latexminted/cmdline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@


class ArgParser(argparse.ArgumentParser):
def __init__(self, *, prog: str):
def __init__(self, *, prog: str, **kwargs):
Copy link

@drin drin Dec 20, 2025

Choose a reason for hiding this comment

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

Suggested change
def __init__(self, *, prog: str, **kwargs):
def __init__(self, *, prog: str, allow_abbrev: bool, formatter_class, **kwargs):
"""Cooperative init method, but provided `allow_abbrev` and `formatter_class` args will be ignored."""

Copy link

@drin drin Dec 20, 2025

Choose a reason for hiding this comment

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

not sure I got the spacing right, but if you add these to the parameter list they won't be propagated to kwargs, and then they can be safely ignored (which would address muzimuzhi's comment). I added the docstring to make it clear they will be ignored, but that it's cooperative otherwise

super().__init__(
prog=prog,
allow_abbrev=False,
formatter_class=argparse.RawTextHelpFormatter
formatter_class=argparse.RawTextHelpFormatter,
**kwargs
)
Comment on lines 24 to 31
Copy link
Contributor

@muzimuzhi muzimuzhi Oct 12, 2025

Choose a reason for hiding this comment

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

(Most likely, usages of ArgParser class in this project will never feel the limitation discussed below.)

This way introduces a new limitation: allow_abbrev and formatter_class cannot appear in kwargs. Thus ArgParser(prog='prog', allow_abbrev=True) will raise an error.

My proposal in #463 (comment) doesn't have such limitation. Using kwargs.update(...) overwrites possibly existing values in kwargs, while kwargs.setdefault(...) only sets default values.

Copy link
Author

@ZhongRuoyu ZhongRuoyu Oct 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, yes I understand this. I opted to keep this simpler fix because ArgParser is internal to this source file and its usage never passes these. If allow_abbrev or formatter_class appearing in kwargs is something that needs to be anticipated, then the .update() approach that overwrites values may not be desired either?

The issue with color (which this PR attempts to fix) is: color is a new keyword argument and argparse is setting it to a default value if it is not yet set 1. I don't anticipate allow_abbrev or formatter_class to be set by argparse, and if it's to be set by a future programmer in this project, I'd actually prefer an explicit TypeError rather than silently overriding their intent with kwargs.update().

Footnotes

  1. https://github.com/python/cpython/blob/v3.14.0/Lib/argparse.py#L1250-L1252 ↩

Copy link

@drin drin Dec 20, 2025

Choose a reason for hiding this comment

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

I suggested a change that will ignore these args (as in it'll capture them into variables that are then unused) and added a docstring that explicitly says they'll be ignored. IMO it's cleaner than using update or setdefault even though it's more verbose

self._prog = prog
self._command_subparsers = None
Expand Down