TST: fix casting extension tests for external users #41278
Conversation
jorisvandenbossche
added
the
Testing
| def test_astype_string(self, data, nullable_string_dtype): | ||
| # GH-33465 | ||
| from pandas.core.arrays.string_arrow import ArrowStringDtype # noqa: F401 |
There was a problem hiding this comment.
To force register the dtype, I think (the same reason that @simonjayhawkins also has been adding it in several places). AFAIU, once the ArrowStringDtype is merged with StringDtype, that will be done automatically and all those imports can be removed.
There was a problem hiding this comment.
yep. these are removed in #39908
There was a problem hiding this comment.
why can't you do the import IN the fixture itself? having in the test is very odd (when you also have the fixture)
There was a problem hiding this comment.
There is no fixture to import it inside, only a pytest.mark.parametrize decorator
There was a problem hiding this comment.
we do this type of thing in the parquet tests, need to do the same here
There was a problem hiding this comment.
Can you be more specific as "this type of thing in the parquet tests" ?
the nullable_string_dtype fixtures, sure there is
The whole point of this PR is to not use that, please look at the diff. I am removing the nullable_string_dtype fixture with a local parametrization
There was a problem hiding this comment.
why, this is exactly the reason this fixture should exist. to avoid having to do a local import like this. which will surely be done incorrectly the next time. a fixture exists for exactly this reason.
There was a problem hiding this comment.
The local imports are only temporary, they will be removed in #39908
|
Failure is the flaky ResourceWarning |
cc @simonjayhawkins fixtures defined by pandas are not known when inheriting the tests as external package. There are a bunch of fixtures that external packages are expected to define in
pandas/tests/extension/conftest.py, but I think for the string dtype here this is not worth it to expect downstream users to define it themselves (certainly given that it is still going to change)