Skip to content

Conversation

mroeschke
Copy link

@martinfleis
Copy link
Member

Thanks! I am fine with this right now but I'm wondering if it is a right solution. Assuming there's a number of downstream packages implementing EAs, this would need to be done in every one of them. Wouldn't it be better to ensure that EA tests just work without a need to define generic fixtures for every EA implementation? I would assume that you only override those that are affected by the specifics of your EA and inherit the rest but that is not the case at the moment.

@jorisvandenbossche
Copy link
Member

Yes, we have always ensured on the pandas side that the base extension array tests don't use other fixtures than the ones defined in those tests itself. I think we should continue on that path (otherwise it is very hard to know as downstream user which fixtures should be implemented).

I think we can simply revert the small part of pandas-dev/pandas#56583 that touches the extension tests. Ideally we would have a better test on the pandas side to remind us of this contract, so we don't easily break that with such "cleaning" PRs.

@mroeschke
Copy link
Author

Yeah agreed a better long term solution is needed on the pandas side to avoid this. Opened up pandas-dev/pandas#56735 to track this

@jorisvandenbossche
Copy link
Member

I did a PR to pandas to fix it there, and we have the issue to add better testing for this. That resolves this for now on the geopandas side, so closing this PR.

@mroeschke mroeschke deleted the tst/pandas_ea_testing branch January 16, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants