10000 Added get_shape as an alias for get_size + tests by saranti · Pull Request #25425 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Added get_shape as an alias for get_size + tests #25425

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Mar 17, 2023
Merged

Added get_shape as an alias for get_size + tests #25425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
41aba54
Added get_shapes method as alias to get_size
saranti Mar 10, 2023
bfd3ccf
merge tests and add exception test
saranti Mar 11, 2023
86c5063
match the correct runtime error
saranti Mar 14, 2023
ca54ced
remove reduntant lines by making get_size call get_shape
saranti Mar 16, 2023
076e3c1
simplify further
saranti Mar 16, 2023
34dc0da
fix previous commit error
saranti Mar 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions lib/matplotlib/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ def __init__(self, ax,

def __str__(self):
try:
size = self.get_size()
return f"{type(self).__name__}(size={size!r})"
shape = self.get_shape()
return f"{type(self).__name__}(shape={shape!r})"
except RuntimeError:
return type(self).__name__

Expand All @@ -286,10 +286,16 @@ def __getstate__(self):

def get_size(self):
"""Return the size of the image as tuple (numrows, numcols)."""
return self.get_shape()[:2]

def get_shape(self):
"""
Return the shape of the image as tuple (numrows, numcols, channels).
"""
if self._A is None:
Copy link
Member

Choose a reason for hiding this comment

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

You could save having the RuntimeError repeated by making get_size call get_shape?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Left comments about testing, strongly prefer the exception gets tested but won't block merging over that.

I'm fine either way with merging the tests (but have enough of a preference to comment on it).

Thanks for the feedback, @jklymak @tacaswell. I decided to merge the tests and add the exception test.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see that you actually did this....

raise RuntimeError('You must first set the image array')

return self._A.shape[:2]
return self._A.shape

def set_alpha(self, alpha):
"""
Expand Down
12 changes: 12 additions & 0 deletions lib/matplotlib/tests/test_image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1468,3 +1468,15 @@ def test__resample_valid_output():
resample(np.zeros((9, 9), np.uint8), np.zeros((9, 9)))
with pytest.raises(ValueError, match="must be C-contiguous"):
resample(np.zeros((9, 9)), np.zeros((9, 9)).T)


def test_axesimage_get_shape():
# generate dummy image to test get_shape method
ax = plt.gca()
im = AxesImage(ax)
with pytest.raises(RuntimeError, match="You must first set the image array"):
im.get_shape()
z = np.arange(12, dtype=float).reshape((4, 3))
im.set_data(z)
assert im.get_shape() == (4, 3)
Copy link
Member

Choose a reason for hiding this comment

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

On one hand, this is testing two things so not technically a unit test. On the other hand all of our tests have a high aspect of integration testing to them (e.g. this test invokes plt.gca()!) so the trade off on one less slightly more complex test seems worth it to me, but fine either way.

Suggested change
assert im.get_shape() == (4, 3)
assert im.get_shape() == (4, 3)
assert im.get_size() == im.get_shape()

assert im.get_size() == im.get_shape()
0