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

Add __call__ As Alias For RandomVariable sample Method #253

Conversation

AFg6K7h4fhy2
Copy link
Collaborator

@AFg6K7h4fhy2 AFg6K7h4fhy2 commented Jul 9, 2024

This PR is aimed at cleaning up code in tutorials, tests, and elsewhere. The RandomVariable class in metaclass.py has an abstract sample method. Subclasses of RandomVariable typically are sampled. By having an alias for sample in __call__, MSR users need not continually write RV.sample(), as this will be done automatically.

@AFg6K7h4fhy2 AFg6K7h4fhy2 changed the title Add __call__ As Alias For RandomVariable sample Method Add __call__ As Alias For RandomVariable sample Method Jul 9, 2024
@AFg6K7h4fhy2 AFg6K7h4fhy2 self-assigned this Jul 9, 2024
@AFg6K7h4fhy2 AFg6K7h4fhy2 added the clean up Good code that could be better label Jul 9, 2024
@AFg6K7h4fhy2 AFg6K7h4fhy2 added this to the N Sprint milestone Jul 9, 2024
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.73%. Comparing base (7bd2250) to head (903fc2d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #253      +/-   ##
==========================================
+ Coverage   92.69%   92.73%   +0.03%     
==========================================
  Files          40       40              
  Lines         904      908       +4     
==========================================
+ Hits          838      842       +4     
  Misses         66       66              
Flag Coverage Δ
unittests 92.73% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@AFg6K7h4fhy2 AFg6K7h4fhy2 marked this pull request as ready for review July 15, 2024 16:07
Copy link
Collaborator

@damonbayer damonbayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @AFg6K7h4fhy2! There are some more places where explicit sample() calls could be replaced with the what you have done in this PR (for example, in the two models). Do you think we should pursue this in this PR or elsewhere?

@AFg6K7h4fhy2
Copy link
Collaborator Author

Thanks, @AFg6K7h4fhy2! There are some more places where explicit sample() calls could be replaced with the what you have done in this PR (for example, in the two models). Do you think we should pursue this in this PR or elsewhere?

A silly mistake was made. See DMs. I will cover the rest of these here.

@AFg6K7h4fhy2 AFg6K7h4fhy2 requested a review from sbidari July 15, 2024 19:56
@damonbayer
Copy link
Collaborator

@AFg6K7h4fhy2 Please review my changes from 903fc2d. If you approve of them (informally), I think this can be merged.

Copy link
Collaborator Author

@AFg6K7h4fhy2 AFg6K7h4fhy2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I support your changes @damonbayer

@damonbayer damonbayer self-requested a review July 16, 2024 14:12
@damonbayer damonbayer merged commit fb8a8a4 into main Jul 16, 2024
8 checks passed
@damonbayer damonbayer deleted the 226-UPX3-add-__call__-method-for-randomvariable-class-that-is-an-alias-for-sample branch July 16, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean up Good code that could be better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add __call__ method for RandomVariable class that is an alias for sample
2 participants