Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Document and test _get_packed_offsets()#14516
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| @@ -81,6 +117,9 @@ def _get_packed_offsets(wd_list, total, sep, mode="fixed"): | |||
| elif mode == "equal": | |||
| maxh = max(w_list) | |||
| if total is None: | |||
| if sep is None: | |||
| raise ValueError("total and sep cannot both be None when " | |||
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.
This raised a TypeError before in the line below (int + None).
Since the function is private, we can do the change without announcement.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-Authored-By: Elliott Sales de Andrade <quantum.analyst@gmail.com>
| @@ -123,4 +125,58 @@ def test_get_packed_offsets(wd_list, total, sep, mode): | |||
| # issue tickets (at least #10476 and #10784) related to corner cases | |||
| # triggered inside this function when calling higher-level functions | |||
| # (e.g. `Axes.legend`). | |||
| # These are just some additional smoke tests. The output is untested. | |||
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.
It looks like this test could just be deleted as it is essentially covered by the tests below?
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.
Not exactly, e.g. I do not test negativetotal orsep. I agree that they'd better be tested explicitly for their output, but until then this is better than nothing.
Since it is not documented whatshould happen in this cases, I've left this as is (unresolved). IMHO that discussion is worth some thought and a separate PR.
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.
Feel free to merge when fixed.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| ---------- | ||
| wd_list : list of (float, float) | ||
| (width, xdescent) of boxes to be packed. | ||
| total : float or None |
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.
| total :floatorNone | |
| total :floator*None* |
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.
Generally, I don't think we style parameter types.
Uh oh!
There was an error while loading.Please reload this page.
Co-Authored-By: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-Authored-By: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-Authored-By: Elliott Sales de Andrade <quantum.analyst@gmail.com>
PR Summary
Properly test and document
matplotlib.offsetbox._get_packed_offsets().PR Checklist