-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Return an empty string for empty model representation #5577
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5577 +/- ##
=======================================
Coverage 87.58% 87.58%
=======================================
Files 76 76
Lines 13702 13704 +2
=======================================
+ Hits 12001 12003 +2
Misses 1701 1701
|
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.
Thanks for opening the PR, I left a suggestion below. Also we should add a test to make sure the original error no longer happens
pymc/printing.py
Outdated
@@ -58,6 +58,9 @@ def str_for_dist(rv: TensorVariable, formatting: str = "plain", include_params: | |||
def str_for_model(model: Model, formatting: str = "plain", include_params: bool = True) -> str: | |||
"""Make a human-readable string representation of Model, listing all random variables | |||
and their distributions, optionally including parameter values.""" | |||
if len(model.unobserved_RVs) + len(model.observed_RVs) + len(model.potentials) == 0: |
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.
Instead of adding this verbose if statement
you can check if rv_reprs
below is an empty list before going into the the if "latex"... else
statement.
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.
Thanks for the feedback. I have one question. There doesn't seem to be any existing test for this function so I am not sure in which file to write the test. Should I create a new test_printing file for just this one test or is there any existing file where I can add it?
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.
Good question. That relates to #5579, in the meantime you can perhaps add it to test_model.py
?
Thanks @sagartomar! |
… instead of raising an error
Should fix #5568
Thank your for opening a PR!
Depending on what your PR does, here are a few things you might want to address in the description: