-
Notifications
You must be signed in to change notification settings - Fork 45
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
Update benchmarks for Python 3.12 #148
Conversation
writer.headers = ["Module"] + (["Call"] if include_call else []) + formatted_python_versions + [f"Relative slowdown (versus {baseline_module}, latest Python)"] | ||
writer.type_hints = [pytablewriter.String] * len(writer.headers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytablewriter
v1.0.0 dropped these, so I replaced them with the new names: thombashi/pytablewriter@7777033
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a huge deal, but on the second table the extra column makes the table wider than the page will go (and looks cut-off):
The relative speeds are probably most important to at a glance, so maybe those should be the first column before the python versions? (Absolutely not a strong opinion here, just figured I'd mention it)
@AlecRosenbaum Very true. I've switched the order of the columns so that the relative speed-up is the second column. Aside: for my future reference: As for making the columns thinner, I thought about changing .. table::
+--------------------------------------------------------+
| Result |
+========================================================+
|.. raw:: html |
| |
| <span title="Incorrect Result (``None``)">❌</span> |
+--------------------------------------------------------+ Which works, but but |
d5ab26d
to
90543e8
Compare
@@ -23,7 +23,7 @@ def __str__(self): | |||
if self.exception: | |||
return f"Raised ``{self.exception}`` Exception" | |||
elif not self.matched_expected: | |||
return f"**Incorrect Result** (``{self.parsed_value}``)" | |||
return "❌" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reduced width so that the columns all fit without a scroll bar.
But of course, the table width shown in PR review is not the same as shown on the project main page 🤦: so the column re-ordering will be necessary (unless we drop some columns) |
What are you trying to accomplish?
Updating the benchmarking metrics in the tables.
What approach did you choose and why?
Ran the benchmarking scripts.
I had to exclude
pendulum
andmaya
(which usespendulum
) sincependulum
won't build on Python 3.12 until v3.0.0 comes out (any day now???): python-pendulum/pendulum#696tox
no longer supports Python < 3.7, so it isn't useful for that anymore.This means that the Python 2.7 data hasn't been updated. 😞
What should reviewers focus on?
There should be no changes to the Python 2.7 column. I had to manually insert this data, so that's the spot that's most likely to have broken, though I've double-checked.
The impact of these changes
Users can see our Python 3.12 performance numbers and feel that we are keeping up with modern Python.