Skip to content
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.

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

Improve requirements and environment markers parsing #44

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

sdispater
Copy link
Member

This PR changes the parser we use to parse requirements and environment markers.

We drop pyparsing for https://github.com/lark-parser/lark which gives a 3 to 4 times boost in performance. This is especially true for complex markers.

  • Added tests for changed code.
  • Updated documentation for changed code.

Copy link
Member

@abn abn left a comment

Choose a reason for hiding this comment

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

vendor.txt needs to be updated. We should also drop 2.7 from the python version string from vendor/pyproject.toml so the lock file is cleaner.

Otherwise lark looks good. Should marker related code be in it's own package rather than under version?

Also, can we drop pyparsing.py from _vendor?

@sdispater sdispater force-pushed the improve-requirement-and-markers-parsing branch 2 times, most recently from ee0af29 to 341fb3d Compare July 24, 2020 11:59
@sdispater sdispater force-pushed the improve-requirement-and-markers-parsing branch from 341fb3d to 24ed1cd Compare July 24, 2020 12:03
@sdispater sdispater requested a review from abn July 24, 2020 12:04
@sdispater
Copy link
Member Author

I'll go ahead and merge this so that it can be tested in the next beta release of Poetry.

@sdispater sdispater merged commit 98365a9 into master Jul 24, 2020
@sdispater sdispater deleted the improve-requirement-and-markers-parsing branch July 24, 2020 13:21
@sdispater sdispater mentioned this pull request Jul 24, 2020
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