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

API: Rename keepdim kwarg to keepdims #1008

Merged
merged 18 commits into from
Apr 24, 2023
Merged

Conversation

neosunhan
Copy link
Collaborator

@neosunhan neosunhan commented Aug 18, 2022

Description

Renames keepdim kwarg to keepdims, in accordance with the numpy API.

Affected functions:

  • prod
  • sum
  • all
  • any
  • max
  • median
  • min
  • percentile
  • vecdot
  • logsumexp

Issue/s resolved: #1006

Changes proposed:

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Due Diligence

  • All split configurations tested
  • Multiple dtypes tested in relevant functions
  • Documentation updated (if needed)
  • Updated changelog.md under the title "Pending Additions"

Does this change modify the behaviour of other functions? If so, which?

no

skip ci

@ghost
Copy link

ghost commented Aug 18, 2022

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@codecov
Copy link

codecov bot commented Sep 13, 2022

Codecov Report

Merging #1008 (6b8a18f) into main (cb46e2c) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1008   +/-   ##
=======================================
  Coverage   91.77%   91.77%           
=======================================
  Files          72       72           
  Lines       10485    10485           
=======================================
  Hits         9623     9623           
  Misses        862      862           
Flag Coverage Δ
unit 91.77% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
heat/cluster/_kcluster.py 90.09% <100.00%> (ø)
heat/cluster/kmeans.py 97.05% <100.00%> (ø)
heat/cluster/kmedians.py 71.69% <100.00%> (ø)
heat/cluster/kmedoids.py 76.92% <100.00%> (ø)
heat/core/_operations.py 96.04% <100.00%> (ø)
heat/core/arithmetics.py 98.59% <100.00%> (ø)
heat/core/linalg/basics.py 95.44% <100.00%> (ø)
heat/core/logical.py 100.00% <100.00%> (ø)
heat/core/statistics.py 96.88% <100.00%> (ø)
heat/naive_bayes/gaussianNB.py 93.58% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ClaudiaComito ClaudiaComito marked this pull request as ready for review February 9, 2023 11:12
@ClaudiaComito ClaudiaComito added this to the 1.3.0 milestone Mar 29, 2023
@ClaudiaComito ClaudiaComito self-assigned this Apr 17, 2023
@ClaudiaComito ClaudiaComito changed the title Rename keepdim kwarg API: Rename keepdim kwarg to keepdims Apr 17, 2023
@github-actions
Copy link
Contributor

Thank you for the PR!

@ClaudiaComito
Copy link
Contributor

@neosunhan @mtar I'm done here. I've renamed keepdim in Heat reduce operations as well, which called for more refactoring in a number of modules.

Thanks again @neosunhan for the great work!

@ClaudiaComito ClaudiaComito assigned mtar and unassigned ClaudiaComito Apr 18, 2023
@ClaudiaComito ClaudiaComito requested a review from mtar April 18, 2023 08:34
@mtar mtar removed their assignment Apr 19, 2023
@mtar mtar merged commit eaa90d6 into main Apr 24, 2023
@mtar mtar deleted the api/1006-rename-keepdim-keepdims branch April 24, 2023 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename keepdim to keepdims
3 participants