Add casting keyword to numeric array types. #547
Conversation
Codecov Report
@@ Coverage Diff @@
## master #547 +/- ##
==========================================
+ Coverage 72.99% 73.16% +0.16%
==========================================
Files 51 51
Lines 6474 6473 -1
Branches 1302 1301 -1
==========================================
+ Hits 4726 4736 +10
+ Misses 1349 1342 -7
+ Partials 399 395 -4
Continue to review full report at Codecov.
|
|
@mdickinson This is exactly what I was asking for in #1244 😄 |
There was a problem hiding this comment.
LGTM.
I think @mdickinson wanted to look into this too.
This makes the coerce attribute entirely an no-opt (but it will still be defined, and will need to be for backward compatibility). It could be a separate PR to update the documentation and things.
|
Thank you! Fixed the merge conflict |
traits/trait_numeric.py
Outdated
| @@ -351,13 +365,29 @@ class CArray(AbstractArray): | |||
| second dimension must be at least 2.) | |||
| value : numpy array | |||
| A default value for the array. | |||
| casting : str | |||
| Casting rule for ``numpy.ndarray.astype``. Values that cannot be | |||
There was a problem hiding this comment.
Can we reword this docstring a bit to say what the effect of this keyword is, so that the reader doesn't have to go digging into the NumPy documentation to figure it out?
It may also be worth clarifying that this parameter is not used if dtype is not given.
There was a problem hiding this comment.
I was thinking of something along the lines of:
Rule used to convert array values to the target dtype. The default is
"unsafe", which
allows arbitrary data conversions."safe"permits only conversions that don't change
the value (so for example conversion fromfloat64tofloat32would not be permitted).
The meaning and behaviour are exactly as innumpy.ndarray.astype. see the NumPy
documentation for other options for this parameter.
There was a problem hiding this comment.
For bonus points, we could even make this a proper Sphinx link and add NumPy to the intersphinx configuration.
| shape=None, | ||
| value=None, | ||
| typecode=None, | ||
| casting="unsafe", |
There was a problem hiding this comment.
Consider making this parameter keyword-only (here and elsewhere)? I don't think there's any good case for passing this by position.
|
LGTM! |
This adds a
castingkeyword to numpy array types. This is passed toastypeif the specified and incoming value's dtype do not match. This allows the user to, for example, only allow values to be set that can be safely cast (casting="safe"). Currently"unsafe"is used as it'sastype's default.The "
if self.coerce:" block is removed from withinAbstractArrayas I believe it was nonfunctional. Within the containing "if ... value.dtype != self.dtype:" block, it only seems to control whether or not the stored array must be a copy of the incoming value in the case that the incoming value is already a numpy array and the dtypes matched (asarrayvsastype). However, that block is only entered if the dtypes don't match, so this case would not come up.