ENH: Implement EA unary ops #23313

sinhrks · 2018-10-24T09:32:03Z

closes Implement/dispatch unary operations for ExtensionArrays #23087
tests added / passed
- Array
- Series
- DataFrame
- Panel
- Sparse
- Extension (Decimal)
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Changed to apply unary funcs per blocks. As a result, unexpected dtype changes are also fixed (as below).

# Current master (-pd.DataFrame({'a': [1], 'b': [1.1]})).dtypes # a float64 # b float64 # dtype: object # After the PR (-pd.DataFrame({'a': [1], 'b': [1.1]})).dtypes # a int64 # b float64 # dtype: object

pep8speaks · 2018-10-24T09:32:23Z

Hello @sinhrks! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file pandas/tests/extension/base/__init__.py:

Line 53:80: E501 line too long (83 > 79 characters)

Comment last updated at 2019-06-30 19:12:01 UTC

TomAugspurger · 2018-10-25T11:18:18Z

pandas/core/internals/blocks.py

 regex=regex, convert=convert)

+ def apply(self, func, **kwargs):
+ # neg should be treated as inv


Do we want this? NumPy seemed to regret that decision, at least for boolean ndarrays.

This is commonly used in internal also. Should be deprecated in 0.24?

TomAugspurger · 2018-10-25T11:21:17Z

pandas/core/arrays/base.py

+ `op` cannot be stored in the ExtensionArray. The dtype of the
+ ndarray uses NumPy's normal inference rules.
+
+ Example


Should be Examples with an s

pandas/core/arrays/base.py

TomAugspurger · 2018-10-25T11:26:05Z

pandas/core/generic.py

- raise TypeError("Unary negative expects numeric dtype, not {}"
- .format(values.dtype))
- return self.__array_wrap__(arr)
+ new_data = self._data.apply(operator.neg)


cc @jreback @jbrockmendel do we want to use Block.apply here? I don't recall what the current guidelines are for ops.

We've gotten arithmetic/comparison ops out of internals. Haven't done anything to unary ops.

Thx, I'm going to move the logic to ops.py immitating binary ops manner.

pandas/tests/series/test_arithmetic.py

jbrockmendel · 2018-10-26T01:09:14Z

pandas/tests/series/test_arithmetic.py

+ # GH#23087
+ ser = Series([1.1, 3.1, 2.1], index=range(3), dtype=typ)
+
+ if op is operator.inv:


simpler to do operator.inv in a separate test and leave it out of the parameterize

jorisvandenbossche · 2018-10-26T10:41:22Z

pandas/core/ops.py

+ return result.__finalize__(self)
+ else:
+ with np.errstate(all='ignore'):
+ new_data = op(self.values)


Should this be self._values to make sure it is called on the ExtensionArray ?

jorisvandenbossche · 2018-10-26T10:42:43Z

pandas/tests/arrays/test_integer.py

+class TestUnaryOps(object):
+
+ @pytest.mark.parametrize('typ', ['Int64', 'Int32', 'Int16', 'Int8',
+ 'UInt64', 'UInt32', 'UInt16', 'UInt8'])


there is a fixture defined for this at the top of the file you can reuse

pandas/core/ops.py

jbrockmendel · 2018-10-30T17:21:14Z

pandas/core/ops.py

+ return operator.inv(values)
+ except Exception:
+ # inv fails with 0 len
+ if not np.prod(values.shape):


values.size? Any more specific exception?

pandas/core/arrays/sparse.py

jreback · 2018-12-03T02:00:11Z

@sinhrks can you rebase and will see where this PR is.

jreback · 2018-12-27T22:27:54Z

can you merge master and upate

codecov · 2019-01-11T02:31:56Z

Codecov Report

Merging #23313 into master will decrease coverage by 0.17%.
The diff coverage is 98.7%.

@@ Coverage Diff @@ ## master #23313 +/- ## ========================================== - Coverage 92.38% 92.2% -0.18%  ========================================== Files 166 162 -4 Lines 52321 51738 -583 ========================================== - Hits 48337 47706 -631  - Misses 3984 4032 +48

Flag	Coverage Δ
#multiple	`90.6% <98.7%> (-0.21%)`	⬇️
#single	`43.05% <57.14%> (-0.02%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/arrays/base.py	`97.04% <100%> (-1.22%)`	⬇️
pandas/core/frame.py	`96.91% <100%> (-0.02%)`	⬇️
pandas/core/generic.py	`96.57% <100%> (-0.06%)`	⬇️
pandas/core/arrays/integer.py	`95.66% <100%> (-0.66%)`	⬇️
pandas/core/arrays/sparse.py	`91.84% <100%> (-0.33%)`	⬇️
pandas/core/ops.py	`94.4% <97.29%> (+0.11%)`	⬆️
pandas/io/s3.py	`0% <0%> (-86.37%)`	⬇️
pandas/io/sas/sasreader.py	`86.2% <0%> (-9.95%)`	⬇️
pandas/io/parquet.py	`76.92% <0%> (-7.7%)`	⬇️
... and 77 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d46d2a5...15fef47. Read the comment docs.

codecov · 2019-01-11T02:32:11Z

Codecov Report

❗ No coverage uploaded for pull request base (master@734b6d1). Click here to learn what that means.
The diff coverage is 98.73%.

@@ Coverage Diff @@ ## master #23313 +/- ## ========================================= Coverage ? 91.73% ========================================= Files ? 178 Lines ? 50810 Branches ? 0 ========================================= Hits ? 46609 Misses ? 4201 Partials ? 0

Flag	Coverage Δ
#multiple	`90.32% <98.73%> (?)`
#single	`41.25% <60.75%> (?)`

Impacted Files	Coverage Δ
pandas/core/arrays/base.py	`99.47% <100%> (ø)`
pandas/core/arrays/timedeltas.py	`88.86% <100%> (ø)`
pandas/core/frame.py	`96.88% <100%> (ø)`
pandas/core/arrays/datetimes.py	`97.8% <100%> (ø)`
pandas/core/generic.py	`93.98% <100%> (ø)`
pandas/core/arrays/integer.py	`96.4% <100%> (ø)`
pandas/core/arrays/sparse.py	`93.48% <100%> (ø)`
pandas/core/ops.py	`94.8% <97.14%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 734b6d1...536d002. Read the comment docs.

sinhrks · 2019-01-15T09:44:36Z

Sorry to take a long. Updated tests to meet NumPy changes.

One remaining error in Azure is caused by network connection which looks unrelated to the change.

jreback

looks really good @sinhrks

@TomAugspurger @jbrockmendel if you'd have a look. I am inclined for 0.24.0.

jreback · 2019-01-16T02:06:57Z

pandas/tests/frame/test_arithmetic.py

+# Unary
+class TestFrameUnary(object):
+
+ @pytest.mark.parametrize('typ', ['int64', 'int32', 'int16', 'int8',


you can import these from pandas.conftest (we only have fixtures for the current int types, not the EA ones, though you might want to create one for combined int/EA types and use it here)

done. Please suggest if there is better arg name.

jreback · 2019-01-16T02:07:44Z

pandas/tests/series/test_arithmetic.py

+ tm.assert_series_equal(-(ser < 0), ~(ser < 0))
+
+ @pytest.mark.parametrize('typ', ['int64', 'int32', 'int16', 'int8',
+ 'uint64', 'uint32', 'uint16', 'uint8',


same comment as above

jreback · 2019-01-16T02:08:08Z

pandas/tests/series/test_arithmetic.py

+ ser = Series([pd.Timestamp('2011-01-01'), pd.Timestamp('2011-01-02'),
+ pd.Timestamp('2011-01-03')], index=range(3), dtype=typ)
+ # pandas all raises
+ pytest.raises(TypeError, op, ser)


can you use the other form

with pytest.raises(TypeError): op(ser)

jreback · 2019-01-26T15:40:16Z

@sinhrks if you can move the whatsnew to 0.25.0

jreback · 2019-04-05T00:58:08Z

can you merge master and update

jreback · 2019-06-08T20:17:56Z

@pandas-dev/pandas-core if anyone would like to rebase this, very close to being ready

WillAyd

OK @jreback got this to green. Definitely a few rough edges but lmk what you think needs to happen to get this merged

WillAyd · 2019-06-12T01:10:35Z

pandas/tests/series/test_arithmetic.py

+ op(ser.values)
+ else:
+ result = op(ser)
+ if op is operator.pos and pd.compat.numpy._is_numpy_dev:


Not sure how this is going to hold up. w.r.t the linked issue:

numpy/numpy#11450

That was milestoned for numpy 1.16 but interestingly only appears in dev with this change.

jbrockmendel · 2019-08-12T18:09:34Z

pandas/conftest.py


+@pytest.fixture(params=['__pos__', '__neg__', '__inv__', '__invert__',
+ '__abs__'])
+def all_unary_operators(request):


can this have a name reflecting the fact that these are the strings and not operator.pos, operator.neg, etc?

jbrockmendel · 2019-08-12T18:10:33Z

pandas/core/arrays/timedeltas.py


 # Note: TimedeltaIndex overrides this in call to cls._add_numeric_methods
+ def __pos__(self):
+ return self.copy()


should this be a copy or view?

jbrockmendel · 2019-08-12T18:10:48Z

pandas/core/arrays/datetimes.py

+ def __pos__(self):
+ raise TypeError("Unary plus expects numeric dtype, not {}"
+ .format(self.dtype))
+


same for PeriodArray?

jbrockmendel · 2019-08-12T18:11:30Z

pandas/core/ops.py



+# ----------------------------------------------------------------------
+# Unary


id be on board with an ops.unary file

jbrockmendel · 2019-08-12T18:12:14Z

@sinhrks can you rebase

WillAyd · 2019-08-28T16:22:21Z

Nice PR but I think has gone stale so closing for now - ping if you'd like to pick this back up

sinhrks force-pushed the unary branch from 0ec907c to 2a49713 Compare October 25, 2018 09:24

TomAugspurger reviewed Oct 25, 2018

View reviewed changes

TomAugspurger added Numeric Operations Arithmetic, Comparison, and Logical operations ExtensionArray Extending pandas with custom dtypes or arrays. labels Oct 25, 2018

jreback reviewed Oct 26, 2018

View reviewed changes

pandas/tests/series/test_arithmetic.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Oct 26, 2018

View reviewed changes

jorisvandenbossche reviewed Oct 26, 2018

View reviewed changes

sinhrks force-pushed the unary branch 3 times, most recently from 6500c9b to 6f64b96 Compare October 30, 2018 10:05

jbrockmendel reviewed Oct 30, 2018

View reviewed changes

sinhrks force-pushed the unary branch from 6f64b96 to d77a058 Compare October 31, 2018 01:12

TomAugspurger reviewed Nov 2, 2018

View reviewed changes

pandas/core/arrays/sparse.py Show resolved Hide resolved

sinhrks force-pushed the unary branch 2 times, most recently from a9fdbd8 to 31ae035 Compare December 6, 2018 05:31

sinhrks force-pushed the unary branch from 31ae035 to 15fef47 Compare January 11, 2019 02:31

sinhrks force-pushed the unary branch 5 times, most recently from 346db01 to 5f1f1df Compare January 15, 2019 08:14

sinhrks changed the title ~~WIP: ENH: Implement EA unary ops~~ ENH: Implement EA unary ops Jan 15, 2019

jreback requested changes Jan 16, 2019

View reviewed changes

jreback added this to the 0.24.0 milestone Jan 16, 2019

WillAyd added 16 commits June 8, 2019 16:30

Merge remote-tracking branch 'upstream/master' into unary

af84599

Reverted bad merge of 24 whatsnew

b0975f4

Removed Panel-specific code

8e3638c

Added stdlib operator import

63430e7

Merge remote-tracking branch 'upstream/master' into unary

9fe6085

Removed old np13 check

b6430f1

Fixed wrong error msg expectation

736fc27

lint fixup

87b8cab

Merge remote-tracking branch 'upstream/master' into unary

c6437ba

Added warnings catch for operator pos

91a7fec

lint fixup

b221fd0

Reverted warnings catch

0afef3c

Reimplemented warnings catch for np dev

4c322b6

test fixup

2b480be

Used OrderedDict for py35 compat

ad62830

Changed to tm.assert_produces_warning

0d6e0c5

WillAyd reviewed Jun 12, 2019

View reviewed changes

WillAyd added 2 commits June 30, 2019 14:08

Merge remote-tracking branch 'upstream/master' into unary

4f6d656

Fixed bad imports

536d002

jbrockmendel reviewed Aug 12, 2019

View reviewed changes

WillAyd closed this Aug 28, 2019

TomAugspurger mentioned this pull request Jan 15, 2020

Incorrect boolean bitwise conversion when dtype=object #31035

Open



		# ----------------------------------------------------------------------
		# Unary

Uh oh!

ENH: Implement EA unary ops #23313

ENH: Implement EA unary ops #23313

Uh oh!

Conversation

sinhrks commented Oct 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

pep8speaks commented Oct 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2019-06-30 19:12:01 UTC

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented Dec 3, 2018

jreback commented Dec 27, 2018

codecov bot commented Jan 11, 2019

Codecov Report

codecov bot commented Jan 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

sinhrks commented Jan 15, 2019

jreback left a 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

jreback commented Jan 26, 2019

jreback commented Apr 5, 2019

jreback commented Jun 8, 2019

WillAyd left a 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

jbrockmendel commented Aug 12, 2019

WillAyd commented Aug 28, 2019

Labels

7 participants

sinhrks commented Oct 24, 2018 •

edited

Loading

pep8speaks commented Oct 24, 2018 •

edited

Loading

codecov bot commented Jan 11, 2019 •

edited

Loading