Skip to content

refactor(pypi): cleanup marker evaluation code in requirement parsing#3765

Open
aignas wants to merge 4 commits intobazel-contrib:mainfrom
aignas:aignas.refactor.parse_requirements_evaluate_markers
Open

refactor(pypi): cleanup marker evaluation code in requirement parsing#3765
aignas wants to merge 4 commits intobazel-contrib:mainfrom
aignas:aignas.refactor.parse_requirements_evaluate_markers

Conversation

@aignas
Copy link
Copy Markdown
Collaborator

@aignas aignas commented May 10, 2026

Another cleanup PR to make the code easier to work with and optimize.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the PyPI requirement parsing logic to evaluate environment markers inline during the parsing phase, removing the need for the separate evaluate_markers function and its associated dependencies. The changes simplify the hub_builder and pip_repository implementations by passing platform environment information directly to parse_requirements. A review comment identified several performance bottlenecks in the new implementation, specifically regarding redundant requirement parsing and argument processing within nested loops, and provided a code suggestion to optimize these operations.

Comment thread python/private/pypi/parse_requirements.bzl Outdated
aignas added 2 commits May 10, 2026 22:49
…f platform loop

Address review feedback: pre-parse requirements once per file instead
of per-entry per-platform, hoist argparse calls outside the platform
loop, and move platforms.get(plat) outside the entry loop.
@aignas aignas marked this pull request as ready for review May 10, 2026 13:50
@aignas aignas requested a review from rickeylev as a code owner May 10, 2026 13:50
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.

1 participant