Skip to content

Conversation

frankie567
Copy link
Contributor

@frankie567 frankie567 commented Dec 20, 2019

Change Summary

I came across this while reading the benchmark page in the docs: benchmark for django-rest-framework was named django-restful-framework. This confused me a bit.

I changed the package name in the benchmark script and run the script instead of modifying directly the generated MD file, as specified.

Problem is that, on my machine, the benchmark is not in favor of pydantic 😟So I don't know what you prefer for this (I would be okay to just edit the previous results).

Related issue number

Nope.

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)

@codecov
Copy link

codecov bot commented Dec 20, 2019

Codecov Report

Merging #1119 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1119   +/-   ##
=======================================
  Coverage   99.97%   99.97%           
=======================================
  Files          20       20           
  Lines        3395     3395           
  Branches      664      664           
=======================================
  Hits         3394     3394           
  Misses          1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7ef2d5...288b784. Read the comment docs.

@dmontagu
Copy link
Contributor

Did you cythonize before running the benchmark locally? That will make pydantic about 2x faster.

@samuelcolvin
Copy link
Member

Also, I always use the same cases.json for consistency, you can download it from here.


I know that .benchmarks_table.md says "do not edit", but I think here for simplicity you can just do a find a replace on "django-restful-framework" and thereby edit both files without regenerating benchmarks.

@frankie567
Copy link
Contributor Author

@dmontagu I didn't, and TBH I never did that. Shouldn't it be mentioned in the benchmarks page? It just says "run with Python 3.7.4". That doesn't seem very fair regarding the other libs.

@samuelcolvin Ok, I'll do that 👍

@samuelcolvin
Copy link
Member

@dmontagu I didn't, and TBH I never did that. Shouldn't it be mentioned in the benchmarks page? It just says "run with Python 3.7.4". That doesn't seem very fair regarding the other libs.

The benchmarks say:

Benchmarks were run with Python 3.7.4 and the package versions listed above installed via pypi on Ubuntu 18.04.

I think that's the fair way of running benchmarks - with the library versions distributed via pypi.

Pydantic provides binaries compiled with cython from python which is modified in numerous ways to be compilable and as fast as possible when compiled - that's one of the reasons it's fast. If other libraries provided similar binaries for linux, they would be used too; some may already be, I haven't checked.

@frankie567
Copy link
Contributor Author

@samuelcolvin Thank you, I understand better now. So, since I ran the benchmark with a pydantic version pulled from PyPi (not the one on my local machine), I guess then it actually ran with the Cython optimizations?

@samuelcolvin
Copy link
Member

Only if you're on linux, you can checked via print(pydantic.compiled).

@frankie567
Copy link
Contributor Author

I didn't then, i was on macOS. Thank you for providing those details 👍

@samuelcolvin samuelcolvin merged commit e478278 into pydantic:master Dec 20, 2019
@samuelcolvin
Copy link
Member

thanks.

andreshndz pushed a commit to cuenca-mx/pydantic that referenced this pull request Jan 17, 2020
…st-framework (pydantic#1119)

* Rename django-rest-framework benchmark

* Add change file for pydantic#1119

* Rename django-rest-framework in existing benchmarks results
alexdrydew pushed a commit to alexdrydew/pydantic that referenced this pull request Dec 23, 2023
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.

3 participants