-
Notifications
You must be signed in to change notification settings - Fork 4
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
Defer inclusion of generated quantity code #364
Conversation
Code Coverage Summary
Diff against main
Results for commit: 5375571 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 140 suites 6m 28s ⏱️ Results for commit 5375571. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Additional test case details
Results for commit 9e4ef53 ♻️ This comment has been updated with latest results. |
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.
@gowerc Looks ok to me. Should there any new tests or changes to existing tests?
I don't think so as this is more of an internal re-structure rather than any new functionality. I guess you could put in an explicit unit test on the call to I think main thing I was worried about was that the documentation actually makes sense as its hard for me to think what makes intuitive sense to someone who isn't as aware of the project internals. |
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.
Docs read better now @gowerc
I just spotted one typo to fix
Co-authored-by: Isaac Gravestock <[email protected]>
Closes #322
General gist is that all generated quantity code had to be defined in advance even though its not used/compiled until the post processing. This was slightly awkward that it meant the user can't alter the GQ code without re-compiling / re-sampling the whole model and it was in complete contrast to how we handle the link code. This PR attempts to rectify this. All round this isn't that much of a feature change but hopefully just makes the package a tiny bit easier to use with some more separation of concerns.