TYP: hashing #39949

jbrockmendel · 2021-02-21T03:23:57Z

closes #xxxx
tests added / passed
Ensure all linting tests pass, see here for how to run them
whatsnew entry

jbrockmendel · 2021-02-21T17:12:18Z

pandas/core/util/hashing.py

+    return _hash_ndarray(vals, encoding, hash_key, categorize)
+
+
+def _hash_ndarray(


had to separate this out, as i couldn't convince mypy to recognize vals = cast(np.ndarray, vals) or assert isinstance(vals, np.ndarray)

WillAyd

lgtm @simonjayhawkins

jreback · 2021-02-27T19:00:57Z

lgtm. @simonjayhawkins if you have a chance.

jbrockmendel · 2021-03-04T15:43:28Z

if theres no interest here we can close it to clear the queue

jreback · 2021-03-05T01:30:53Z

if theres no interest here we can close it to clear the queue

its fine, this is worth merging.

simonjayhawkins · 2021-03-08T09:46:58Z

pandas/core/frame.py

@@ -9150,7 +9150,7 @@ def count(

        return result.astype("int64")

-    def _count_level(self, level: Level, axis: Axis = 0, numeric_only=False):
+    def _count_level(self, level: Level, axis: int = 0, numeric_only: bool = False):


why change axis? DataFrame._get_axis accepts Axis.

from https://github.com/microsoft/pyright/blob/master/docs/typed-libraries.md#wide-vs-narrow-types

In general, a function input parameter should be annotated with the widest possible type supported by the implementation.

its only called from count, and count calls _get_axis_number just before doing so

simonjayhawkins · 2021-03-08T09:59:14Z

pandas/core/util/hashing.py

@@ -139,25 +167,25 @@ def hash_pandas_object(
    return h


-def hash_tuples(vals, encoding="utf8", hash_key: str = _default_hash_key):
+def hash_tuples(
+    vals: Union[MultiIndex, List[Tuple]],


List -> Iterable

again from https://github.com/microsoft/pyright/blob/master/docs/typed-libraries.md#wide-vs-narrow-types

As a specific application of the “use the widest type possible” rule, libraries should generally use immutable forms of container types instead of mutable forms (unless the function needs to modify the container). Use Sequence rather than List, Mapping rather than Dict, etc.

whats the rule for Sequence vs Iterable?

In this case, we only get here with MultiIndex and List[Tuple]. I get the wider-is-better for public APIs, but for purely internal code strictness seems beneficial. is that distinction not relevant?

AFAIK mypy works by checking each function is isolation. i.e. it checks that all the code within the function is compatible with the function annotation. i.e. checks the types in function calls within the body with the type parameters of the called functions.

so doing the reverse of this, i.e. annotating with the actual types passed seems like extra work imo. (and could be simply achieved using MonkeyType)

Iterable because c_is_list_like accepts an iterable as list-like

Using List is bad practice. function code could modify the contents of the List. If the parameter is typed as an immutable container, mypy won't allow it.

thanks, updated

simonjayhawkins · 2021-03-08T09:59:51Z

pandas/core/util/hashing.py

@@ -139,25 +167,25 @@ def hash_pandas_object(
    return h


-def hash_tuples(vals, encoding="utf8", hash_key: str = _default_hash_key):
+def hash_tuples(
+    vals: Union[MultiIndex, List[Tuple]],


type parameters for Tuple.

simonjayhawkins · 2021-03-08T10:00:20Z

pandas/core/util/hashing.py


    Parameters
    ----------
-    vals : MultiIndex, list-of-tuples, or single tuple
+    vals : MultiIndex or list-of-tuples


list-like of tuples

simonjayhawkins · 2021-03-08T10:00:41Z

pandas/core/util/hashing.py

    """
-    Hash an MultiIndex / list-of-tuples efficiently
+    Hash an MultiIndex / list-of-tuples efficiently.


list-like-of-tuples

pandas/core/util/hashing.py

simonjayhawkins · 2021-03-08T10:12:22Z

pandas/core/util/hashing.py

@@ -224,11 +252,11 @@ def _hash_categorical(c, encoding: str, hash_key: str):


 def hash_array(
-    vals,
+    vals: ArrayLike,


vals in docstring also needs updating.

simonjayhawkins · 2021-03-08T10:16:00Z

pandas/core/util/hashing.py

@@ -255,11 +283,26 @@ def hash_array(
    # hash values. (This check is above the complex check so that we don't ask
    # numpy if categorical is a subdtype of complex, as it will choke).
    if is_categorical_dtype(dtype):
+        vals = cast("Categorical", vals)
        return _hash_categorical(vals, encoding, hash_key)
    elif is_extension_array_dtype(dtype):
        vals, _ = vals._values_for_factorize()
        dtype = vals.dtype


dtype = vals.dtype not now needed?

good catch, will update

simonjayhawkins

Thanks @jbrockmendel merge on green

* TYP: hashing * platform compat, mypy fixup * setops fix * port more annotations * update annotations, docstring * List-> Iterable

jbrockmendel added 2 commits February 20, 2021 19:20

TYP: hashing

cd2ca27

platform compat, mypy fixup

ae9cc28

jbrockmendel commented Feb 21, 2021

View reviewed changes

setops fix

60e2c4f

jreback added the Typing type annotations, mypy/pyright type checking label Feb 23, 2021

jreback added this to the 1.3 milestone Feb 23, 2021

WillAyd approved these changes Feb 26, 2021

View reviewed changes

jbrockmendel added 2 commits March 7, 2021 14:12

Merge branch 'master' into typ-hashing

c3115e9

port more annotations

89becd9

simonjayhawkins reviewed Mar 8, 2021

View reviewed changes

jbrockmendel added 3 commits March 8, 2021 08:08

Merge branch 'master' into typ-hashing

a813783

update annotations, docstring

9ab4be5

List-> Iterable

c423757

simonjayhawkins approved these changes Mar 8, 2021

View reviewed changes

jbrockmendel merged commit c29facc into pandas-dev:master Mar 8, 2021

jbrockmendel deleted the typ-hashing branch March 8, 2021 18:12

jbrockmendel added a commit to jbrockmendel/pandas that referenced this pull request Mar 11, 2021

TYP: hashing (pandas-dev#39949)

2e5f359

* TYP: hashing * platform compat, mypy fixup * setops fix * port more annotations * update annotations, docstring * List-> Iterable

simonjayhawkins mentioned this pull request Jun 15, 2021

BUG: 1.3.0rc1 pd.util.hash_array on Index fails to access _values_for_factorize #42003

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TYP: hashing #39949

TYP: hashing #39949

jbrockmendel commented Feb 21, 2021

jbrockmendel Feb 21, 2021

WillAyd left a comment

jreback commented Feb 27, 2021

jbrockmendel commented Mar 4, 2021

jreback commented Mar 5, 2021

simonjayhawkins Mar 8, 2021

jbrockmendel Mar 8, 2021

simonjayhawkins Mar 8, 2021

jbrockmendel Mar 8, 2021

simonjayhawkins Mar 8, 2021

jbrockmendel Mar 8, 2021

simonjayhawkins Mar 8, 2021

simonjayhawkins Mar 8, 2021

simonjayhawkins Mar 8, 2021

simonjayhawkins Mar 8, 2021

simonjayhawkins Mar 8, 2021 •

edited

Loading

jbrockmendel Mar 8, 2021

simonjayhawkins left a comment

		return _hash_ndarray(vals, encoding, hash_key, categorize)


		def _hash_ndarray(

TYP: hashing #39949

TYP: hashing #39949

Conversation

jbrockmendel commented Feb 21, 2021

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

jreback commented Feb 27, 2021

jbrockmendel commented Mar 4, 2021

jreback commented Mar 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins Mar 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins left a comment

Choose a reason for hiding this comment

simonjayhawkins Mar 8, 2021 •

edited

Loading