-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
ENH: Ensure that repr and str work for MaskedArray non-ndarray bases #4576
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
@mhvk I'll try to get to this and the previous PR tomorrow when I'm fresh. |
@@ -3612,19 +3611,19 @@ def __repr__(self): | |||
|
|||
""" | |||
n = len(self.shape) | |||
name = repr(self._data).split('(')[0] | |||
name = ('array' if self.__class__ == np.ndarray else |
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.
Don't need the parenthesis here. Might read better if it was an ordinary if statement.
Looks OK. Man, there is a lot of obfuscated code in ma. The use of lower case aliases for class names is a prime example. And the singleton |
@charris - thanks for the comments. I changed to an if statement, and in the process discovered that it was incorrect as written anyway, so that the name would change for an |
Great. Could you now squash the commits and begin the summary line |
For repr, use the name of the base class in output as "masked_<name>" (with name=array for ndarray to match the previous implementation). For str, insert masked_print_option in an ndarray view of the object array that is created for string output, to avoid calling __setitem__ in the base class. Add tests to ensure this works.
@charris - OK, made everything into a single commit, and added a line to the release notes. Will expand this line in further PRs. |
Looks good. Thanks @mhvk . |
ENH: Ensure that repr and str work for MaskedArray non-ndarray bases
(@charris: this is one of the corrections I mentioned in #3907 that we need to make to MaskedArray to get astropy's more complicated ndarray subclasses to be used as base classes.)
Currently,
repr(ma)
andstr(ma)
do not deal well with base classes that redefine__repr__
to use a different way to print the class name, or that override__setitem__
to do checks on values, respectively. This PR corrects this, by simply letting__repr__
use__class__.__name__
instead of trying to be smart in how to get it, and by ensuring that__str__
insertsmasked_print_option
in anndarray
view of the object array that is created for string output (the current way__str__
is handled is perhaps too clever a trick, but I'm not quite sure yet how to make it safer; my PR should at least help a little.).Specifically, currently, after defining:
one gets
With the PR,