-
Notifications
You must be signed in to change notification settings - Fork 2
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
perf: Optimize the CLI by completely avoiding Pydantic imports from everyvoice -h
.
#429
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #429 +/- ##
==========================================
- Coverage 76.02% 75.98% -0.04%
==========================================
Files 42 42
Lines 2757 2757
Branches 455 455
==========================================
- Hits 2096 2095 -1
- Misses 576 577 +1
Partials 85 85 ☔ View full report in Codecov by Sentry. |
What about following the convention in utils with type_definitions_heavy.py or something like that? Ideally we should use the same naming convention as the division in utils in any case. |
That sounds better, I'll take a look.
✔️💯 |
Alright, investigation done, |
527a3a7
to
75a65db
Compare
@roedoejet OK, file renamed, and rebased onto main, so this is ready to re-review and possibly merge. |
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.
lgtm
75a65db
to
8898f3b
Compare
PR Goal?
Get the CLI even faster, by avoiding all pydantic imports. Prompted once again by typing
everyvoice new<tab>
and being annoyed at the delay getting results.According to CI this PR brings
everyvoice -h
down from .28s to .21s, but on GPSCC I'm seeing .65 down to .50 so I care more, and where I also really wish I could optimize further...Fixes?
My impatience. Well, no, it doesn't fix my impatience, it only satisfies a little bit of it. 😄
Feedback sought?
I'm not sure about the name
fs2/types_expensive.py
. First I called itfs2/shared_types.py
to parallel what we do in ev/config, but it's not shared so that makes no sense.fs2/type_definitions.py
was nicely self-documenting, and I feel I'm partially undoing that, so suggestions for a better name are very welcome.Priority?
Normal
Tests added?
Should already be covered by CI.
How to test?
time everyvoice -h
before and after the PR.Confidence?
High for the code changes, low for the file name.
Version change?
no