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

docs(performance): remove section on literal vs enum performance #9262

Merged
merged 1 commit into from Apr 19, 2024

Conversation

ollz272
Copy link
Contributor

@ollz272 ollz272 commented Apr 16, 2024

Change Summary

I've removed the docs on the performance differences between using a literal or an enum. It seems the performance improvements made in 2.7 have made them perform nearly identically in the previous example, so unless theres a case missing, it doesn't seem worth promoting this tip anymore.

Related issue number

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @davidhewitt

Copy link

codspeed-hq bot commented Apr 16, 2024

CodSpeed Performance Report

Merging #9262 will not alter performance

Comparing ollz272:docs-change (ec56183) with main (ae71183)

Summary

✅ 13 untouched benchmarks

@ollz272
Copy link
Contributor Author

ollz272 commented Apr 16, 2024

So for some benchmarks, using the following script:

import enum
from timeit import timeit

from typing_extensions import Literal

from pydantic import TypeAdapter

ta = TypeAdapter(Literal['a', 'b'])
result1 = timeit(lambda: ta.validate_python('a'), number=1000000)


class AB(str, enum.Enum):
    a = 'a'
    b = 'b'


ta = TypeAdapter(AB)
result2 = timeit(lambda: ta.validate_python('a'), number=1000000)
print(result2 / result1)

on 2.6.4 -> 2.80
on 2.7.0 -> 0.99

Im using a 2022 M2 macbook, maybe results differ on other platforms.

Copy link
Contributor

@ybressler ybressler left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the detailed performance comment.

(Also, I'm not a maintainer, just reviewing some PRs informationally)

@ollz272
Copy link
Contributor Author

ollz272 commented Apr 17, 2024

please review

@sydney-runkle
Copy link
Member

@ollz272,

Nice work! Thanks!!

@sydney-runkle sydney-runkle added the relnotes-ignore Omit this PR from the release notes. label Apr 19, 2024
@sydney-runkle sydney-runkle merged commit bb857bd into pydantic:main Apr 19, 2024
53 of 55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review relnotes-ignore Omit this PR from the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants