Skip to content

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Feb 13, 2024

The all_parsers fixture has this check, but some of the other fixtures were missing it.

  • [n/a] closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • [n/a] Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

@QuLogic
Copy link
Contributor Author

QuLogic commented Feb 13, 2024

Given #54466, this is primarily targeted at getting backported to 2.2.x.

def _validate(self, data) -> None:
dtype = data.dtype
if not isinstance(dtype, ArrowDtype):
if not pa_version_under10p1 and not isinstance(dtype, ArrowDtype):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only unsure about this one; ArrowDtype is conditionally imported in this file, but I don't know if this should be if pa_version_under10p1 or not isinstance9dtype, ArrowDtype):?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this this was OK as is?

  • If the type wasn't an ArrowDtype this should raise as the wrong type
  • If the type was ArrowDtype, a user already needed the correct version of pyarrow as it's validated in the ArrowDtype.__init__

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, it's not OK as is. ArrowDtype is conditionally imported, so its use here needs to be guarded. Otherwise you get a test failure:

____________________ TestSeriesMisc.test_inspect_getmembers ____________________
[gw0] linux -- Python 3.12.2 /usr/bin/python3
self = <pandas.tests.series.test_api.TestSeriesMisc object at 0xb66af1c8>
    def test_inspect_getmembers(self):
        # GH38782
        pytest.importorskip("jinja2")
        ser = Series(dtype=object)
        msg = "Series._data is deprecated"
        with tm.assert_produces_warning(
            DeprecationWarning, match=msg, check_stacklevel=False
        ):
>           inspect.getmembers(ser)
.../pandas/tests/series/test_api.py:178: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/lib/python3.12/inspect.py:607: in getmembers
    return _getmembers(object, predicate, getattr)
/usr/lib/python3.12/inspect.py:585: in _getmembers
    value = getter(object, key)
.../pandas/core/accessor.py:224: in __get__
    accessor_obj = self._accessor(obj)
.../pandas/core/arrays/arrow/accessors.py:73: in __init__
    super().__init__(
.../pandas/core/arrays/arrow/accessors.py:41: in __init__
    self._validate(data)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <pandas.core.arrays.arrow.accessors.ListAccessor object at 0x9924ca38>
data = Series([], dtype: object)
    def _validate(self, data):
        dtype = data.dtype
>       if not isinstance(dtype, ArrowDtype):
E       NameError: name 'ArrowDtype' is not defined
.../pandas/core/arrays/arrow/accessors.py:49: NameError

What I'm unsure of is whether this should raise if (the import condition failed or the type is wrong), or if (the import condition is true and the type is wrong).

@simonjayhawkins simonjayhawkins added Testing pandas testing functions or related to the test suite Arrow pyarrow functionality labels Feb 13, 2024
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Mar 15, 2024
@QuLogic
Copy link
Contributor Author

QuLogic commented Mar 18, 2024

This isn't waiting for me, AFAICT.

@mroeschke
Copy link
Member

Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.

@mroeschke mroeschke closed this Oct 29, 2024
@QuLogic
Copy link
Contributor Author

QuLogic commented Oct 29, 2024

Again, it was waiting for you to reply.

@mroeschke mroeschke reopened this Oct 29, 2024
@mroeschke
Copy link
Member

OK sorry I missed that. Could you resolve the merge conflict?

The `all_parsers` fixture has this check, but some of the other fixtures
were missing it.
@mroeschke
Copy link
Member

It appears another PR fixed the problem this PR was trying to solve. Sorry we weren't able to get this in in the meantime, but closing this PR out.

@mroeschke mroeschke closed this Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arrow pyarrow functionality Stale Testing pandas testing functions or related to the test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants