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.

Sign up for GitHub

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

Jump to bottom

Deprecate sum_kbn and cumsum_kbn? #24804

Closed
nalimilan opened this issue Nov 27, 2017 · 4 comments
Closed

Deprecate sum_kbn and cumsum_kbn? #24804

nalimilan opened this issue Nov 27, 2017 · 4 comments
Labels
deprecation This change introduces or involves a deprecation excision Removal of code from Base or the repository maths Mathematical functions
Milestone
0.7

Comments

@nalimilan
Copy link
Member

It's unusual in the API to provide a separate function just to use a different algorithm. A more general and Julian approach would be to use something like sum(x, alg=KahanBabuskaNeumaier) or sum(KahanBabuskaNeumaier, x) (or just KBN).

Just like there is a separate SortingAlgorithms package, this does not need to live in Base.
@nalimilan nalimilan added the triage This should be discussed on a triage call label Nov 27, 2017
@ararslan ararslan added deprecation This change introduces or involves a deprecation maths Mathematical functions labels Nov 27, 2017
@ViralBShah
Copy link
Member

What are the upvotes for - baking into sum or removing from Base into a package?

@StefanKarpinski
Copy link
Member

Let's just remove it for now, we can create a summation algorithms package or something.

@JeffBezanson
Copy link
Member

Fine to move this out if somebody gets to it but seems pretty non-urgent to me.

@JeffBezanson JeffBezanson added the excision Removal of code from Base or the repository label Nov 30, 2017
@StefanKarpinski
Copy link
Member

We could just delete it and let someone create a package if they really want it.

@StefanKarpinski StefanKarpinski added this to the 1.0 milestone Nov 30, 2017
@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Nov 30, 2017
ararslan added a commit that referenced this issue Nov 30, 2017
@ararslan
Deprecate (cum)sum_kbn
b0aeb49 
This removes `sum_kbn` and `cumsum_kbn` in favor of `sum` and `cumsum`,
respectively, with a note about the potential loss in precision.

Fixes #24804
@ararslan ararslan mentioned this issue Nov 30, 2017
Merged
ararslan added a commit that referenced this issue Nov 30, 2017
@ararslan
Deprecate (cum)sum_kbn
7b3ad39 
This removes `sum_kbn` and `cumsum_kbn` in favor of `sum` and `cumsum`,
respectively, with a note about the potential loss in precision.

Fixes #24804
ararslan added a commit that referenced this issue Dec 2, 2017
@ararslan
Deprecate (cum)sum_kbn
965becf 
This removes `sum_kbn` and `cumsum_kbn` in favor of `sum` and `cumsum`,
respectively, with the `*_kbn` functions moving to a package.

Fixes #24804
ararslan added a commit that referenced this issue Dec 5, 2017
@ararslan
Deprecate (cum)sum_kbn
f7398c0 
This removes `sum_kbn` and `cumsum_kbn` in favor of `sum` and `cumsum`,
respectively, with the `*_kbn` functions moving to a package.

Fixes #24804
ararslan added a commit that referenced this issue Dec 5, 2017
@ararslan
Deprecate (cum)sum_kbn
8992771 
This removes `sum_kbn` and `cumsum_kbn` in favor of `sum` and `cumsum`,
respectively, with the `*_kbn` functions moving to a package.

Fixes #24804
ararslan added a commit that referenced this issue Dec 5, 2017
@ararslan
Deprecate (cum)sum_kbn
be6141a 
This removes `sum_kbn` and `cumsum_kbn` in favor of `sum` and `cumsum`,
respectively, with the `*_kbn` functions moving to a package.

Fixes #24804
ararslan added a commit that referenced this issue Dec 7, 2017
@ararslan
Deprecate (cum)sum_kbn
1a4ebd4 
This removes `sum_kbn` and `cumsum_kbn` in favor of `sum` and `cumsum`,
respectively, with the `*_kbn` functions moving to a package.

Fixes #24804
ararslan added a commit that referenced this issue Dec 8, 2017
@ararslan
Deprecate (cum)sum_kbn (#24869)
5f929a4 
This removes `sum_kbn` and `cumsum_kbn` in favor of `sum` and `cumsum`,
respectively, with the `*_kbn` functions moving to a package.

Fixes #24804
evetion pushed a commit to evetion/julia that referenced this issue Dec 12, 2017
@ararslan @evetion
Deprecate (cum)sum_kbn (JuliaLang#24869)
f15c987 
This removes `sum_kbn` and `cumsum_kbn` in favor of `sum` and `cumsum`,
respectively, with the `*_kbn` functions moving to a package.

Fixes JuliaLang#24804
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
deprecation This change introduces or involves a deprecation excision Removal of code from Base or the repository maths Mathematical functions
Projects
None yet
Milestone
0.7
Development

No branches or pull requests
5 participants
@StefanKarpinski @ViralBShah @JeffBezanson @nalimilan @ararslan