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.

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

Jump to bottom

Add tomli_loads benchmark #200

Merged
merged 12 commits into from
Apr 7, 2023
Merged

Add tomli_loads benchmark #200

merged 12 commits into from
Apr 7, 2023

Conversation

kumaraditya303
Copy link
Contributor

No description provided.

@kumaraditya303
Copy link
Contributor Author

It seems like requires-python = ">=3.11" has no effect and seems broken.

@kumaraditya303 kumaraditya303 marked this pull request as ready for review May 14, 2022 08:14
ericsnowcurrently
ericsnowcurrently requested changes May 24, 2022
View reviewed changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! It mostly LGTM. There's only one thing I'd like added.

pyperformance/data-files/benchmarks/bm_tomllib_loads/run_benchmark.py Outdated Show resolved Hide resolved
ericsnowcurrently
ericsnowcurrently approved these changes May 27, 2022
View reviewed changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for working on this.

pyperformance/data-files/benchmarks/bm_tomllib_loads/generate_data.py Outdated Show resolved Hide resolved
@ericsnowcurrently ericsnowcurrently removed their assignment Jun 10, 2022
@ericsnowcurrently
Copy link
Member

We can merge this once the tests are passing. Do you know what the problem is?

@kumaraditya303
Copy link
Contributor Author

kumaraditya303 commented Jun 10, 2022 •
edited
Loading

See #200 (comment)

The issue is that requires-python metadata has no effect.

@ericsnowcurrently
Copy link
Member

Ah, right. I'll take a look at that real quick and then merge it.

@mdboom
Copy link
Contributor

mdboom commented Jun 17, 2022

Maybe we should have this install toml from PyPI instead of only using the stdlib one? That would give us more Python version coverage.

@hauntsaninja
Copy link
Contributor

(tomllib is based on pypi's tomli, not toml)

@AlexWaygood AlexWaygood mentioned this pull request Mar 1, 2023
Open
@kumaraditya303 kumaraditya303 changed the title Add tomllib.loads benchmark Add tomli_loads benchmark Apr 7, 2023
@kumaraditya303 kumaraditya303 merged commit 9c58774 into python:main Apr 7, 2023
@kumaraditya303 kumaraditya303 deleted the tomllib branch April 7, 2023 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kumaraditya303 @ericsnowcurrently @mdboom @hauntsaninja