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

BUG: series sum changes dtype uint64 to int64 #53418

Conversation

srkds
Copy link
Contributor

@srkds srkds commented May 27, 2023

🚫Before
image

✅After changes
image

@topper-123
Copy link
Contributor

Thanks, @srkds.

This should be handled in _get_dtype_max, unless I'm missing something . You should also add tests for this and an entry in the v2.1 whatsnew.

@topper-123 topper-123 added Bug Numeric Operations Arithmetic, Comparison, and Logical operations labels May 27, 2023
@topper-123 topper-123 added this to the 2.1 milestone May 27, 2023
@srkds
Copy link
Contributor Author

srkds commented May 28, 2023

I missed the _get_dtype_max method while checking the execution flow. Thanks for the suggestion 👍
Added test case and entry in whatsnew file.

Thanks!

Copy link
Contributor

@topper-123 topper-123 left a comment

Choose a reason for hiding this comment

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

Generally looks good, just a single comment.

s = Series([10000000000000000000], dtype="uint64")
result = s.sum()
expected = np.uint64(10000000000000000000)
tm.assert_almost_equal(result, expected)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move the test over to pandas.tests.reductions.test_reductions.TestSeriesReductions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes! That seems right place.

Copy link
Contributor

@topper-123 topper-123 left a comment

Choose a reason for hiding this comment

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

LGTM.

@mroeschke mroeschke merged commit d8ae9a7 into pandas-dev:main May 30, 2023
34 checks passed
@mroeschke
Copy link
Member

Thanks @srkds

@topper-123
Copy link
Contributor

Nice one, @srkds.

@srkds
Copy link
Contributor Author

srkds commented May 31, 2023

Thanks @topper-123 and @mroeschke for reviewing and merging.
I have made progress from contributing to docs fixing typos, then adding test cases, and now contributed a bug fix 🎉😀.
It wouldn't be possible without maintainers like you.
Looking forward to contributing more ❤.

topper-123 added a commit to topper-123/pandas that referenced this pull request May 31, 2023
topper-123 pushed a commit to topper-123/pandas that referenced this pull request Jun 5, 2023
* BUG:series sum changes dtype uint64 to int64

* condition check should be at _get_dtype_max

* add test for series uint64 sum method

* DOC: add entry in whatsnew v2.1.0

* series tests should be at test_reductions file
Daquisu pushed a commit to Daquisu/pandas that referenced this pull request Jul 8, 2023
* BUG:series sum changes dtype uint64 to int64

* condition check should be at _get_dtype_max

* add test for series uint64 sum method

* DOC: add entry in whatsnew v2.1.0

* series tests should be at test_reductions file
mroeschke pushed a commit that referenced this pull request Jul 13, 2023
* ENH: better dtype inference when doing DataFrame reductions

* precommit issues

* fix failures

* fix failures

* mypy + some docs

* doc linting linting

* refactor to use _reduce_with_wrap

* docstring linting

* pyarrow failure + linting

* pyarrow failure + linting

* linting

* doc stuff

* linting fixes

* fix fix doc string

* remove _wrap_na_result

* doc string example

* pyarrow + categorical

* silence bugs

* silence errors

* silence errors II

* fix errors III

* various fixups

* various fixups

* delay fixing windows and 32bit failures

* BUG: Adding a columns to a Frame with RangeIndex columns using a non-scalar key (#52877)

* DOC: Update whatsnew (#52882)

* CI: Change development python version to 3.10 (#51133)

* CI: Change development python version to 3.10

* Update checks

* Remove strict

* Remove strict

* Fixes

* Add dt

* Switch python to 3.9

* Remove

* Fix

* Try attribute

* Adjust

* Fix mypy

* Try fixing doc build

* Fix mypy

* Fix stubtest

* Remove workflow file

* Rename back

* Update

* Rename

* Rename

* Change python version

* Remove

* Fix doc errors

* Remove pypy

* Update ci/deps/actions-pypy-39.yaml

Co-authored-by: Thomas Li <47963215+lithomas1@users.noreply.github.com>

* Revert pypy removal

* Remove again

* Fix

* Change to 3.9

* Address

---------

Co-authored-by: Thomas Li <47963215+lithomas1@users.noreply.github.com>

* update

* update

* add docs

* fix windows tests

* fix windows tests

* remove guards for 32bit linux

* add bool tests + fix 32-bit failures

* fix pre-commit failures

* fix mypy failures

* rename _reduce_with -> _reduce_and_wrap

* assert missing attributes

* reduction dtypes on windows and 32bit systems

* add tests for min_count=0

* PERF:median with axis=1

* median with axis=1 fix

* streamline Block.reduce

* fix comments

* FIX preserve dtype with datetime columns of different resolution when merging (#53213)

* BUG Merge not behaving correctly when having `MultiIndex` with a single level (#53215)

* fix merge when MultiIndex with single level

* resolved conversations

* fixed code style

* BUG: preserve dtype for right/outer merge of datetime with different resolutions (#53233)

* remove special BooleanArray.sum method

* remove BooleanArray.prod

* fixes

* Update doc/source/whatsnew/v2.1.0.rst

Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>

* Update pandas/core/array_algos/masked_reductions.py

Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>

* small cleanup

* small cleanup

* only reduce 1d

* fix after #53418

* update according to comments

* revome note

* update _minmax

* REF: add keepdims parameter to ExtensionArray._reduce + remove ExtensionArray._reduce_and_wrap

* REF: add keepdims parameter to ExtensionArray._reduce + remove ExtensionArray._reduce_and_wrap

* fix whatsnew

* fix _reduce call

* simplify test

* add tests for any/all

---------

Co-authored-by: Patrick Hoefler <61934744+phofl@users.noreply.github.com>
Co-authored-by: Thomas Li <47963215+lithomas1@users.noreply.github.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Yao Xiao <108576690+Charlie-XIAO@users.noreply.github.com>
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Numeric Operations Arithmetic, Comparison, and Logical operations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Pandas uint64 sums to int64
3 participants