[ArrayManager] TST: get tests running for /tests/frame #39700
Conversation
jorisvandenbossche
added
the
Internals
lithomas1
added
the
Testing
jorisvandenbossche
force-pushed
the
am-tests-frame
branch
from
a7b1551 to
b0d8ff8
Compare
jorisvandenbossche
force-pushed
the
am-tests-frame
branch
from
b0d8ff8 to
016ce1a
Compare
|
Rebased this now #39841 is merged |
| @@ -165,7 +166,10 @@ def test_constructor_cast_failure(self): | |||
| df["foo"] = np.ones((4, 2)).tolist() | |||
|
|
|||
| # this is not ok | |||
| msg = "Wrong number of items passed 2, placement implies 1" | |||
| msg = ( | |||
There was a problem hiding this comment.
can you use the "|".join pattern
There was a problem hiding this comment.
Yes, but I already did that in #39991, which I would like to get merged first, and then will fix the conflics with this PR to resolve this
| # GH#12857 | ||
| lets = list("ACDEFGHIJKLMNOP") | ||
| slen = 50 | ||
| nseqs = 1000 | ||
| words = [[np.random.choice(lets) for x in range(slen)] for _ in range(nseqs)] | ||
| df = DataFrame(words).astype("U1") | ||
| assert (df.dtypes == object).all() | ||
| # TODO(Arraymanager) astype("U1") actually gives this dtype instead of object |
There was a problem hiding this comment.
im pretty sure we dont want this behavior? so should xfail for now?
There was a problem hiding this comment.
This test is not actually testing astype, but rather repr, so I would prefer to run the rest of the test (which actually passes) without having this line error.
Now, I assume we should have an astype-specific test about this as well, that could be xfailed.
There was a problem hiding this comment.
I don't directly see a test about it in frame/methods/test_astype.py, so I will add a test for that there. It's not very clear, though, what the expected behaviour should be.
There was a problem hiding this comment.
could move the repr calls up and do the dtype assertion at the end
| @@ -896,8 +902,13 @@ def verify(df): | |||
| ) | |||
|
|
|||
| right = DataFrame(vals, columns=cols, index=idx) | |||
| if using_array_manager: | |||
| # INFO(ArrayManager) with ArrayManager preserve dtype where possible | |||
There was a problem hiding this comment.
is INFO(ArrayManager) a pattern i should know?
There was a problem hiding this comment.
Yeah, it was requested by Jeff on one of the previous PRs. It's an explicit comment about behaviour that changed with ArrayManager, but which is not a TODO (since it's not wrong behaviour that still needs to be fixed)
|
couple comments, otherwise LGTM |
| @@ -816,7 +816,13 @@ def insert(self, loc: int, item: Hashable, value, allow_duplicates: bool = False | |||
|
|
|||
| value = extract_array(value, extract_numpy=True) | |||
| if value.ndim == 2: | |||
| value = value[0, :] | |||
| if value.shape[0] == 1: | |||
There was a problem hiding this comment.
maybe make a helper function for this type of operation / check (followon ok)
|
thanks, hopefully nothing breaks from prior merges :-< |
xref #39146
This is on top of #39612, so only the last commit is relevant for this PR: ec83091