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

ENH: add np.nan* funcs to cython_table #22109

Merged

Conversation

topper-123
Copy link
Contributor

This PR ensures that Series.agg can correctly take np.nan* funcs as inputs, e.g.:

>>> pd.Series([1,2,3,4]).agg(np.nansum)
10

Previously that operation returned a Series. See #19629 for details.

Tests are added through the use of _get_cython_table_params in the parametrisation of test_agg_cython_table test function in series/test_apply/frame/test_apply.

@topper-123 topper-123 force-pushed the add_np.nanfunc_to_cython_table branch 3 times, most recently from e85a39d to 449d912 Compare July 28, 2018 23:05
@topper-123
Copy link
Contributor Author

The failure is caused by a bug introduced in #21224. A fix has been proposed in #22110.

@topper-123 topper-123 force-pushed the add_np.nanfunc_to_cython_table branch from 449d912 to b12371d Compare July 29, 2018 10:35
@codecov
Copy link

codecov bot commented Jul 29, 2018

Codecov Report

Merging #22109 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #22109   +/-   ##
=======================================
  Coverage   92.22%   92.22%           
=======================================
  Files         169      169           
  Lines       51261    51261           
=======================================
  Hits        47277    47277           
  Misses       3984     3984
Flag Coverage Δ
#multiple 90.66% <ø> (ø) ⬆️
#single 42.23% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/base.py 97.61% <ø> (ø) ⬆️

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 da9d851...51198ad. Read the comment docs.

@jreback jreback added Enhancement Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Jul 29, 2018
@jreback jreback added this to the 0.24.0 milestone Jul 29, 2018
@jreback
Copy link
Contributor

jreback commented Jul 29, 2018

so couple of things on this. I don't the the additional functions are actually tested when run in pandas/tests/frame/test_apply.py. Its also very hard to see as the ids are not specified when actually running these tests, can you fixup?

@jreback
Copy link
Contributor

jreback commented Sep 25, 2018

@topper-123 can you rebase

@jreback
Copy link
Contributor

jreback commented Oct 18, 2018

can you rebase

@topper-123 topper-123 force-pushed the add_np.nanfunc_to_cython_table branch from b12371d to 7080d24 Compare October 27, 2018 18:42
@pep8speaks
Copy link

Hello @topper-123! Thanks for updating the PR.

@topper-123 topper-123 force-pushed the add_np.nanfunc_to_cython_table branch from 7080d24 to 4dc9980 Compare October 27, 2018 21:38
@topper-123 topper-123 force-pushed the add_np.nanfunc_to_cython_table branch from 4dc9980 to 51198ad Compare October 27, 2018 23:01
@topper-123
Copy link
Contributor Author

topper-123 commented Oct 27, 2018

Rebased.

Also, there were xfailed tests in test_agg_category_nansum that now pass. They also just use the fixture from conftest.fy now which is simpler. Not sure if the test should be just deleted.

@jreback jreback merged commit e14c550 into pandas-dev:master Oct 28, 2018
@jreback
Copy link
Contributor

jreback commented Oct 28, 2018

thanks!

thoo added a commit to thoo/pandas that referenced this pull request Oct 30, 2018
…y_tests

* repo_org/master: (52 commits)
  ENH: Allow rename_axis to specify index and columns arguments  (pandas-dev#20046)
  STY: proposed isort settings [ci skip] [skip ci] [ciskip] [skipci] (pandas-dev#23366)
  MAINT: Remove extraneous test.parquet file
  CLN: Follow-up comments to pandas-devgh-23392 (pandas-dev#23401)
  BUG GH23282 calling min on series of NaT returns NaT (pandas-dev#23289)
  unpin openpyxl (pandas-dev#23361)
  REF: collect ops dispatch functions in one place, try to de-duplicate SparseDataFrame methods (pandas-dev#23060)
  CLN: Remove pandas.tools module (pandas-dev#23376)
  CLN: Remove some dtype methods from API (pandas-dev#23390)
  CLN: Cleanup toplevel namespace shims (pandas-dev#23386)
  DOC: fixup whatsnew note for GH21394 (pandas-dev#23355)
  Fix import format at pandas/tests/extension directory (pandas-dev#23365)
  DOC: Remove Series.sortlevel from api.rst (pandas-dev#23395)
  API: Disallow dtypes w/o frequency when casting (pandas-dev#23392)
  BUG/TST/REF: Datetimelike Arithmetic Methods (pandas-dev#23215)
  STYLE: lint
  add np.nan* funcs to cython_table (pandas-dev#22109)
  Run Isort on tests/util single PR (pandas-dev#23347)
  BUG: Fix date_range overflow (pandas-dev#23345)
  Run Isort on tests/arrays single PR (pandas-dev#23346)
  ...
tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018
@topper-123 topper-123 deleted the add_np.nanfunc_to_cython_table branch January 17, 2019 19:03
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

series.agg(np.nansum) etc. returns a series, scalar expected
3 participants