[ArrayManager] TST: include subset of ArrayManager tests in all CI builds #40496
Conversation
jorisvandenbossche
added
Internals
|
@jreback @jbrockmendel as discussed, I am trying here the idea to run a specific subet of tests most relevant for the ArrayManager on each of the builds (right now testing with For that, I added an "arraymanager" mark, and then doing a second pytest run with this mark activated. (there are still some actual failures discovered by this to look at / solve) |
| @@ -926,7 +926,9 @@ def test_setitem_duplicate_columns_not_inplace(self): | |||
| tm.assert_frame_equal(df_view, df_copy) | |||
| tm.assert_frame_equal(df, expected) | |||
|
|
|||
| @pytest.mark.parametrize("value", [1, np.array([[1], [1]]), [[1], [1]]]) | |||
| @pytest.mark.parametrize( | |||
| "value", [1, np.array([[1], [1]], dtype="int64"), [[1], [1]]] | |||
There was a problem hiding this comment.
why?
this is causing me problems in #35417 and i thinking making this change would fix that, but would feel like cheating
There was a problem hiding this comment.
This is just adding a fixed dtype, doesn't change anything else. The default dtype of numpy is platform dependent, while in pandas it's always int64 (as you know ;))
| @@ -905,7 +905,7 @@ def test_unstack_nan_index3(self, using_array_manager): | |||
| if using_array_manager: | |||
| # INFO(ArrayManager) with ArrayManager preserve dtype where possible | |||
| cols = right.columns[[1, 2, 3, 5]] | |||
| right[cols] = right[cols].astype("int64") | |||
| right[cols] = right[cols].astype(df["C"].dtype) | |||
There was a problem hiding this comment.
i take it this is a 32bit thing?
There was a problem hiding this comment.
Yes, it was to fix the windows builds
|
is the test_median failure a false-positive? |
|
No, that was an actual failure (forgot a colon in the |
|
This is all green now. For now, it only adds the /frame/ tests, we can later add the mark to more tests if we want. With only the frame tests, this currently adds eg 135s in the "actions-38.yaml, not slow and not network and not clipboard" build (so 2min to an overall of 42min), which I think is fine. |
Agreed. Do you expect to keep adding to this though? |
Some I suppose, but I think we can evaluate that one at a time when actually adding it whether it adds too much time or not. |
|
looks good, can you rebase once more just to make sure (merged a bunch of things) |
|
Rebased, and all green. |
|
@jorisvandenbossche is this the source of the codecov CI failure? looks like it wasnt green when merged, but the codecov failure may have predated this. |
Ah, yes, sorry, I generally ignore codecov failures since they have been so flaky in the past (but maybe that's better now), but I missed that it wasn't the "patch" coverage that was failing, but the "project" one. I suppose just removing the coverage from the ArrayManager pytest invocation should fix it. Will do a PR. |
xref #39146