np.full now defaults to the filling value's dtype. #7437
Conversation
| array([[10, 10], | ||
| [10, 10]]) | ||
|
|
||
| """ | ||
| if dtype is None: | ||
| dtype = array(fill_value).dtype |
There was a problem hiding this comment.
I wonder about stupid objects, but I guess we just shouldn't care, anyone who wants to do np.full(3, (1, 2)) should be prepared to give dtype=object. (could make it copy=False just for the kicks)
There was a problem hiding this comment.
Well, np.full(3, (1, 2)) (with or without passing dtype=object) didn't work and still doesn't work. The real fun happens when someone write np.full(3, (1, 2, 3)) though (it used to, and still, returns array([1, 2, 3])).
Is there a function equivalent to array(fill_value).dtype, but that would actually return object when a non-scalar is passed in? I thought np.obj2sctype(..., default=object) would work, but it returns the dtype of a ndarray when a ndarray is passed in (so np.full(3, np.array([1, 2, 3])) would still "fail"). (By the way I find obj2sctype's "return None if everything fails" somewhat unpythonic.)
There was a problem hiding this comment.
Forget about this, I guess.... object dtype is funny, and you just have to do it manually. Even noticed we had talked about it before, heh. I guess you are right about obj2sctype, it might be trying to recreate the corresponding C-function, no idea....
There was a problem hiding this comment.
So should we consider that np.full(3, (1, 2, 3)) ==> np.array([1, 2, 3]) (the old behavior, kept in this patch) is not a stopper? I feel like there should be a way to at least error out there.
There was a problem hiding this comment.
Maybe there should be an error. arr.fill kind of tries this, not sure about all the logic it uses.
In any case, this is an orthogonal issue, so I think we should ignore it here.
There was a problem hiding this comment.
Really, the only reason I didn't merge it yet, was that I liked our special tag, like ENH:, or actually I guess here MAINT: is better, in the commit message ;).
|
If you don't mind, could make the commit message "ENH: ..." or similar, we usually try to keep them like that. Anyway, looks good to me, test changes seem good, will merge in a bit if I don't forget. |
0fb7e98 to
be3f71d
Compare
|
Fixed, and added error checking for the object array case (similar to |
|
Thanks, but I am not sure we should put it in here :(, it is again a change in behaviour that should likely have a Deprecation period. Easiest would maybe be to put this in without it now, and open a new PR to deprecate it? If you feel strongly about it, Could make it a deprecation here and we wait if someone else comments. |
be3f71d to
a073198
Compare
|
Undid the previous commit. I'll leave it as it is because I guess it should really be a |
|
Thanks a lot @anntzer. I agree, but easier to do the clear stuf first :). |
MAINT: np.full now defaults to the filling value's dtype.
See #6366.