Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced np.bool refs; fix CI failure #34835

Merged
merged 9 commits into from
Jun 17, 2020

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Jun 16, 2020

I mostly just replace np.bool with np.bool_ though there are some other cases where I just used bool. I don't know if it matters...so happy to conform one way or another

For now just seeing if this fixes CI

Closes #34848.

@jbrockmendel
Copy link
Member

Looks like also need np.complex?

@jreback jreback added the CI Continuous Integration label Jun 17, 2020
@TomAugspurger
Copy link
Contributor

Pushed a batch of np.object -> object. There are still a few more failures.

@jreback jreback added this to the 1.1 milestone Jun 17, 2020
@TomAugspurger TomAugspurger marked this pull request as ready for review June 17, 2020 13:13
@TomAugspurger
Copy link
Contributor

The last run (https://travis-ci.org/github/pandas-dev/pandas/jobs/699299640#L3736) passed, but there were still many warnings. Because we don't run that job with -Werror.

Anyway, hopefully these are all caught now.

@@ -163,7 +163,7 @@ def test_to_records_with_categorical(self):
),
# Pass in a type instance.
(
dict(column_dtypes=np.unicode),
dict(column_dtypes=str),
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we only support python3 str works here.

Comment on lines -703 to -704
"week",
"weekofyear",
Copy link
Contributor

Choose a reason for hiding this comment

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

This fixes warnings coming from pandas. Unrelated to the numpy changes.

@@ -1483,7 +1483,7 @@ def find_common_type(types: List[DtypeObj]) -> DtypeObj:
if has_bools:
for t in types:
if is_integer_dtype(t) or is_float_dtype(t) or is_complex_dtype(t):
return np.object
return object
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return object
return np.dtype("object")

(to ensure this function actually returns a dtype object, as its type annotation indicates)

@@ -264,7 +264,7 @@ def hash_array(

# First, turn whatever array this is into unsigned 64-bit ints, if we can
# manage it.
elif isinstance(dtype, np.bool):
elif isinstance(dtype, bool):
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if this can ever be correct:

In [18]: isinstance(np.dtype(bool), bool) 
Out[18]: False

unless dtype is not actually a dtype ...

@jreback
Copy link
Contributor

jreback commented Jun 17, 2020

after this would be nice to have a lint rule to catch these if possible (as it would fail much sooner than the 3.9 CI)

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jun 17, 2020

Opened #34852 to fail on warnings for that build. I'd like to get CI passing first for other PRs though.

@TomAugspurger
Copy link
Contributor

Good to merge when CI passes?

@TomAugspurger
Copy link
Contributor

Has anyone seen this failure on npdev before? Restarting that build.

=================================== FAILURES ===================================
________________ TestDataFrameSortIndex.test_sort_index_inplace ________________
[gw1] linux -- Python 3.7.7 /home/vsts/miniconda3/envs/pandas-dev/bin/python

self = <pandas.tests.frame.methods.test_sort_index.TestDataFrameSortIndex object at 0x7f566041fa50>

    def test_sort_index_inplace(self):
        frame = DataFrame(
            np.random.randn(4, 4), index=[1, 2, 3, 4], columns=["A", "B", "C", "D"]
        )
    
        # axis=0
        unordered = frame.loc[[3, 2, 4, 1]]
        a_id = id(unordered["A"])
        df = unordered.copy()
        df.sort_index(inplace=True)
        expected = frame
        tm.assert_frame_equal(df, expected)
>       assert a_id != id(df["A"])
E       assert 140008958850064 != 140008958850064
E        +  where 140008958850064 = id(1   -1.103603\n2   -0.373049\n3   -0.418311\n4   -0.079376\nName: A, dtype: float64)

pandas/tests/frame/methods/test_sort_index.py:224: AssertionError

@TomAugspurger
Copy link
Contributor

The 3.9 build passed, with a few warnings I can't track down yet

=============================== warnings summary ===============================

pandas/core/arrays/boolean.py:735

pandas/core/arrays/boolean.py:735

pandas/core/arrays/boolean.py:735

  /home/travis/build/pandas-dev/pandas/pandas/core/arrays/boolean.py:735: DeprecationWarning: In future, it will be an error for 'np.bool_' scalars to be interpreted as an index

    result = op(self._data, other)

pandas/_testing.py:699

pandas/_testing.py:699

pandas/_testing.py:699

pandas/_testing.py:699

pandas/_testing.py:699

pandas/_testing.py:699

pandas/_testing.py:699

pandas/_testing.py:699

  /home/travis/build/pandas-dev/pandas/pandas/_testing.py:699: DeprecationWarning: elementwise comparison failed; this will raise an error in the future.

    diff = np.sum((left.values != right.values).astype(int)) * 100.0 / len(left)

pandas/io/json/_json.py:158: 15 tests with warnings

  /home/travis/build/pandas-dev/pandas/pandas/io/json/_json.py:158: DeprecationWarning: an integer is required (got type float).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.

    return dumps(

pandas/tests/io/json/test_pandas.py:1176

pandas/tests/io/json/test_pandas.py:1176

pandas/tests/io/json/test_pandas.py:1176

  /home/travis/build/pandas-dev/pandas/pandas/tests/io/json/test_pandas.py:1176: DeprecationWarning: an integer is required (got type float).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.

    assert dumps(ts, iso_dates=True) == exp

pandas/tests/io/json/test_pandas.py:1178

pandas/tests/io/json/test_pandas.py:1178

pandas/tests/io/json/test_pandas.py:1178

  /home/travis/build/pandas-dev/pandas/pandas/tests/io/json/test_pandas.py:1178: DeprecationWarning: an integer is required (got type float).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.

    assert dumps(dt, iso_dates=True) == exp

pandas/tests/io/json/test_pandas.py:1202

pandas/tests/io/json/test_pandas.py:1202

pandas/tests/io/json/test_pandas.py:1202

pandas/tests/io/json/test_pandas.py:1202

pandas/tests/io/json/test_pandas.py:1202

pandas/tests/io/json/test_pandas.py:1202

  /home/travis/build/pandas-dev/pandas/pandas/tests/io/json/test_pandas.py:1202: DeprecationWarning: an integer is required (got type float).  Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.

    result = dumps(df, iso_dates=True)

I'll handle those in #34852

@TomAugspurger
Copy link
Contributor

All green @WillAyd @jreback.

@WillAyd
Copy link
Member Author

WillAyd commented Jun 17, 2020

lgtm @jreback . thanks Tom for pushing this through

1 similar comment
@WillAyd
Copy link
Member Author

WillAyd commented Jun 17, 2020

lgtm @jreback . thanks Tom for pushing this through

@eric-wieser
Copy link
Contributor

xref numpy/numpy#14882

@WillAyd WillAyd deleted the no-np-bool branch June 30, 2020 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Failing np.bool on numpy master
6 participants