Skip to content

DRAFT: separate parsing from pip-compile's main function#2374

Draft
sirosen wants to merge 1 commit into
jazzband:mainfrom
sirosen:separate-parsing-phase
Draft

DRAFT: separate parsing from pip-compile's main function#2374
sirosen wants to merge 1 commit into
jazzband:mainfrom
sirosen:separate-parsing-phase

Conversation

@sirosen
Copy link
Copy Markdown
Member

@sirosen sirosen commented Apr 12, 2026

@webknjaz, I'd like to discuss this idea, and I think a draft PR which shows some initial implementation is the best way in this case to kickstart the conversation.
If you prefer, I'm very happy to open a separate issue for us to discuss the proposal.


Separating parsing from handling parsed data is a good practice.
The ever-growing list of arguments to pip-compile is actually a maintenance issue, as there is no modeling of a "CLI request", only a flat dump of all of the options into a shared namespace (inside of the main cli function).

Relatedly, I have some changes I'd like to make to CLI arg handling, and in terms of unit testing, I want to test the results of parsing, rather than testing the underlying system.


This changeset is a draft of a conversion to have a separate module for
the parsing logic for pip-compile. A companion change would be needed
for pip-sync as well.

The overarching goal is that pip-compile's main logic should be
separated from parsing, and that parsing should produce some kind of
well-formed object, rather than a (huge!) tuple of parameters.

There's a simple organization benefit realized immediately from this.
Because the parsing logic is separated out, the main body of
pip-compile becomes much shorter and more readable.

The more profound benefit of this change is that the parser is
independently testable (not yet explored in this work-in-progress). It
should be possible to write numerous tests which have very limited IO or
are even completely sans-IO, and which narrowly assert the behavior of
the parser.

A further benefit which we could explore would be to split up and
categorize the various options into categories. This is done a little bit
for PipArgs, but ExtrasArgs, AnnotationArgs, etc. are possible
directions.

This changeset is a draft of a conversion to have a separate module for
the parsing logic for `pip-compile`. A companion change would be needed
for `pip-sync` as well.

The overarching goal is that `pip-compile`'s main logic should be
separated from parsing, and that parsing should produce some kind of
well-formed object, rather than a (huge!) tuple of parameters.

There's a simple organization benefit realized immediately from this.
Because the parsing logic is separated out, the main body of
`pip-compile` becomes much shorter and more readable.

The more profound benefit of this change is that the parser is
independently testable (not yet explored in this work-in-progress). It
should be possible to write numerous tests which have very limited IO or
are even completely sans-IO, and which narrowly assert the behavior of
the parser.

A further benefit which we could explore would be to split up and
categorize the various options into categories. This is done a little bit
for `PipArgs`, but `ExtrasArgs`, `AnnotationArgs`, etc. are possible
directions.
@sirosen sirosen changed the title DRAFT: separate parsing from 'pip-compile' main DRAFT: separate parsing from pip-compile's main function Apr 12, 2026
@dataclasses.dataclass
class CompileArgs:
# ctx is provided on init only
ctx: dataclasses.InitVar[click.Context]
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.

We'll probably need to add this to ignores in sphinx.. https://app.readthedocs.org/projects/pip-tools/builds/32219714/#315230508--192

"""
This module defines the ``pip-compile`` parser, which consists of two public interfaces:

- ``parse_pip_compile_args``: a decorator which adds parsing to a function.
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.

I think that once Sphinx builds are fixed, this could be referencible via :func:

Copy link
Copy Markdown
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

+1 on the overall approach. I haven't checked every single line, but I trust you to scope the patch appropriately. Approving so that you don't have to wait for another review unless you need one.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants