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

[pythongen] Simplify slot generation iterators #2001

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sneakers-the-rat
Copy link
Collaborator

Noticed that most of the logic of the slot and postinit generation iteration logic in pythongen was just sorting, and also made some potentially expensive calls multiple time. unified a sorting method and trimmed down unused/duplicated code. generated kitchen sink and biolink and both were identical.

sorting usually a lil easier to maintain than truth tables <3.

marginal perf benefit, the lions share of time is still in induced_slot since ig the changes i made there got deployed but we haven't updated the lockfile here yet?

@sneakers-the-rat sneakers-the-rat marked this pull request as draft March 22, 2024 22:37
Copy link

codecov bot commented Mar 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.66%. Comparing base (534a2db) to head (ca9715e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2001      +/-   ##
==========================================
- Coverage   80.69%   80.66%   -0.04%     
==========================================
  Files         104      104              
  Lines       11622    11598      -24     
  Branches     2910     2893      -17     
==========================================
- Hits         9378     9355      -23     
- Misses       1701     1702       +1     
+ Partials      543      541       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sneakers-the-rat
Copy link
Collaborator Author

so i'm trying to make the postinit behavior be identical across classes, and i'm curious - why does most of the validation behavior happen before the super() call?

I would assume that the classes would be structured one of two ways -

  • all slots are rolled down to children, who do all validation except for whatever top-level validation might happen in the root object. all defaults are set by dataclass fields, so are already set by the time the parent tests are called anyway, and so then we want to call all the bottom-level model field validation and coercion last
  • no slots are rolled down to children unless they have something special about them, and so the parent classes handle all the validation of the inherited slots.

why would the super() come at the end? i'm sure i'm missing something

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant