-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Don't pass unused xdescent to _get_packed_offsets. #24462
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
Conversation
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.
Just to make sure: there is no obvious use case where we would like to use xdescent, but just haven't gotten around to do it?
_Params( # total=None (implicit 1) | ||
[(.1, 0)] * 3, total=None, sep=None, expected=(1, [0, .45, .9])), | ||
[.1] * 3, total=None, sep=None, expected=(1, [0, .45, .9])), |
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.
Optional: Write out the list, which is a little more explicit:
[.1] * 3, total=None, sep=None, expected=(1, [0, .45, .9])), | |
[.1, .1, .1], total=None, sep=None, expected=(1, [0, .45, .9])), |
assert result[0] == expected[0] | ||
assert_allclose(result[1], expected[1]) | ||
|
||
|
||
def test_get_packed_offsets_equal_total_none_sep_none(): | ||
with pytest.raises(ValueError): | ||
_get_packed_offsets([(1, 0)] * 3, total=None, sep=None, mode='equal') | ||
_get_packed_offsets([1] * 3, total=None, sep=None, mode='equal') |
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.
as above
_get_packed_offsets([1] * 3, total=None, sep=None, mode='equal') | |
_get_packed_offsets([1, 1, 1], total=None, sep=None, mode='equal') |
@pytest.mark.parametrize('wd_list', | ||
([(150, 1)], [(150, 1)]*3, [(0.1, 1)], [(0.1, 1)]*2)) | ||
@pytest.mark.parametrize('widths', | ||
([150], [150]*3, [0.1], [0.1]*2)) |
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.
as above
([150], [150]*3, [0.1], [0.1]*2)) | |
([150], [150, 150, 150], [0.1], [0.1, 0.1])) |
Instead of passing a list of (widths, xdescents) where xdescent is unused, just pass a list of widths. This helper is private so we just need to adjust the call sites and tests with no deprecation. This patch is preliminary work for some further cleanup on the offsetbox module.
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.
Anybody can merge after CI pass.
Instead of passing a list of (widths, xdescents) where xdescent is unused, just pass a list of widths. This helper is private so we just need to adjust the call sites and tests with no deprecation.
This patch is preliminary work for some further cleanup on the offsetbox module (somewhat motivated by #24386).
PR Summary
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
Release Notes
.. versionadded::
directive in the docstring and documented indoc/users/next_whats_new/
.. versionchanged::
directive in the docstring and documented indoc/api/next_api_changes/
next_whats_new/README.rst
ornext_api_changes/README.rst