TYP Series and DataFrame currently type-check as hashable

[MarcoGorelli]

• closes BUG: Issues with return type hint and/or definition of pandas.core.generic.NDFrame.__hash__ #40013
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry

[mzeitlin11]

@MarcoGorelli does this also take care of #40013?

[MarcoGorelli]

Thanks @mzeitlin11 - indeed, it looks like they're duplicates, I'll close the new one I'd opened

[MarcoGorelli]

@jbrockmendel @jreback I've updated to just use __hash__: None, which is what they do in typeshed (https://github.com/python/typeshed/blob/3fa5988a2ac57cf8ce56e2de171a62b39952f96c/stdlib/builtins.pyi#L602)

The builtin error message will now be shown instead, so I've updated a few tests accordingly. Now, this is what'll be shown:

In [1]: hash(pd.Series([1, 2, 3]))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-1f0c9bc1928b> in <module>
----> 1 hash(pd.Series([1, 2, 3]))

TypeError: unhashable type: 'Series'

instead of

In [1]: hash(pd.Series([1, 2, 3]))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-1f0c9bc1928b> in <module>
----> 1 hash(pd.Series([1, 2, 3]))

~/pandas-marco/pandas/core/generic.py in __hash__(self)
   1874 
   1875     def __hash__(self) -> int:
-> 1876         raise TypeError(
   1877             f"{repr(type(self).__name__)} objects are mutable, "
   1878             f"thus they cannot be hashed"

TypeError: 'Series' objects are mutable, thus they cannot be hashed

[jbrockmendel]

no objection here

[jreback]

ok this seems reasonable. can you add a release note about the changing error message & rebase. ping on green.

[MarcoGorelli]

cool - @jreback done, green

[simonjayhawkins]

Thanks @MarcoGorelli lgtm. do we need tests for Index and EA (or do we have tests and not check the message)?

[simonjayhawkins]

nit.

Suggested change

	- Calling ``hash`` on non-hashable pandas objects will now raise ``TypeError`` with the built-in error message (e.g. ``unhashable type: 'Series'``). Previously it would raise a custom message such as ``"'Series' objects are mutable, thus they cannot be hashed"`` (:issue:`40013`)
	- Calling ``hash`` on non-hashable pandas objects will now raise ``TypeError`` with the built-in error message (e.g. ``unhashable type: 'Series'``). Previously it would raise a custom message such as ``'Series' objects are mutable, thus they cannot be hashed`` (:issue:`40013`)

[simonjayhawkins]

another nit, preference for

        subset_iterable: Iterable
        if subset is None:
            subset_iterable = self.columns
        elif (

to help distinguish cases were we need the variable type annotation (normally where the call to a function returns Any and would be redundant in the future when the return type of the called function is typed) vs a wider type used for a variable than the inferred type( i.e. the return type of the initial assignment to a variable)

[jreback]

@MarcoGorelli some comment and can you rebase

[jreback]

can u rebase and ping on green

[mwaskom]

This is more than a message change because previously isinstance(x, collections.abc.Hashable) would return True and now it will return False, right? (That said, the existing behavior is wrong, so IMO fixing this sooner rather than later would be helpful ... I currently have an awkward workaround to deal with it in a way that mypy understands).

[MarcoGorelli]

This is more than a message change because previously isinstance(x, collections.abc.Hashable) would return True and now it will return False, right?

Indeed, you are correct (I wasn't aware that this would affect that):

# here
In [4]: isinstance(Series([1,2,3]), collections.abc.Hashable)
Out[4]: False

#master
In [2]: isinstance(Series([1,2,3]), collections.abc.Hashable)
Out[2]: True

IMO fixing this sooner rather than later would be helpful

Agreed

[simonjayhawkins]

@MarcoGorelli do we need to add the change of isintance of collections.abc.Hashable?

[simonjayhawkins]

@MarcoGorelli can you merge master to resolve conflicts

[jreback]

@MarcoGorelli if you can merge master

[simonjayhawkins]

mypy will fail.

I think undo all the changes to pandas/core/frame.py

and just do

        if subset is None:
            subset = cast(Sequence[Hashable], self.columns)
        elif (

could maybe add link to #28770 as related.

[simonjayhawkins]

Suggested change

	- Calling ``hash`` on non-hashable pandas objects will now raise ``TypeError`` with the built-in error message (e.g. ``unhashable type: 'Series'``). Previously it would raise a custom message such as ``'Series' objects are mutable, thus they cannot be hashed``. Furthermore, ``isinstance(Series, abc.collections.Hashable)`` will now return ``False`` (:issue:`40013`)
	- Calling ``hash`` on non-hashable pandas objects will now raise ``TypeError`` with the built-in error message (e.g. ``unhashable type: 'Series'``). Previously it would raise a custom message such as ``'Series' objects are mutable, thus they cannot be hashed``. Furthermore, ``isinstance(Series, collections.abc.Hashable)`` will now return ``False`` (:issue:`40013`)

Series is a type so isinstance(Series, collections.abc.Hashable) is True

maybe isinstance(<Series>, collections.abc.Hashable)?

[simonjayhawkins]

and just do

        if subset is None:
            subset = cast(Sequence[Hashable], self.columns)
        elif (

or undo all the changes to frame.py and could simply ignore pandas/core/frame.py:6183: error: Incompatible types in assignment (expression has type "Index", variable has type "Union[Hashable, Sequence[Hashable], None]") [assignment]

and we can remove the ignore once #28770 is fixed (for all pandas objects)

[jreback]

small comment. ping on green.

[jreback]

can eliminate these by either casting (as you do on L6195) or better just to assign to a new variable.

[MarcoGorelli]

@simonjayhawkins do you have a preference here? In #41283 (comment) you'd suggested to put this instead of the cast and to remove it after #28770

[simonjayhawkins]

my preference is for the "fix later" ignore rather than changing code for a potential false positive

[simonjayhawkins]

my preference is for the "fix later" ignore rather than changing code for a potential false positive

[jreback]

thanks @MarcoGorelli

[jreback]

@meeseeksdev backport 1.3.x

[lumberbot-app]

Something went wrong ... Please have a look at my logs.