BUG: fix wrong error message in deprecated 2D indexing of Series with datetime values #38099
Conversation
jorisvandenbossche
added
Regression
| data (values.ndim == 2), which should only be allowed if ndim is | ||
| also 2. | ||
| """ | ||
| if ndim is None: |
There was a problem hiding this comment.
we recently started always-passing ndim to the constructor, so should never have None here
There was a problem hiding this comment.
this is backported though, so prob ok (but will need a patch on 1.2)
There was a problem hiding this comment.
This is also done in the parent method, so can use a general clean-up for 1.2
There was a problem hiding this comment.
kk can you open an issue so we don't forget to do this
|
LGTM pending green. I'm OK with this solution, or with working the check into the existing check_ndim. |
| data (values.ndim == 2), which should only be allowed if ndim is | ||
| also 2. | ||
| """ | ||
| if ndim is None: |
There was a problem hiding this comment.
this is backported though, so prob ok (but will need a patch on 1.2)
| with tm.assert_produces_warning(FutureWarning): | ||
| s[:, None] | ||
| res = series[:, None] |
There was a problem hiding this comment.
prob worth testing the actual message (can pass to assert_produces_warning to prevent regressions.
| s[:, None] | ||
| res = series[:, None] | ||
|
|
||
| assert isinstance(res, np.ndarray) |
There was a problem hiding this comment.
can you assert the result arrays (parameterize if needed)
|
@jorisvandenbossche if you can update |
| ): | ||
| result = series[:, None] | ||
|
|
||
| assert isinstance(result, np.ndarray) |
| result = series[:, None] | ||
|
|
||
| assert isinstance(result, np.ndarray) | ||
| tm.assert_numpy_array_equal(result, np.asarray(series)[:, None]) |
There was a problem hiding this comment.
really pref the
result=
expected=
assert_*
as its what we always want to model
|
@jorisvandenbossche if you can update for the small comments above |
This reverts commit 6391f54.
|
thanks @jorisvandenbossche |
|
@meeseeksdev backport 1.1.x |
This comment has been minimized.
This comment has been minimized.
|
@simonjayhawkins will do the backport |
|
Thanks @jorisvandenbossche |
…exing of Series with datetime values (#38210)
Closes #35527
For most Series types, this was properly raising the deprecation warning about 2D being deprecated, but for Series with datetimetz values, this started raising an AssertionError instead of only raising the warning.