Skip to content

Change bernoulli(1) and bernfrac(1) to +½#639

Closed
Parcly-Taxel wants to merge 10 commits intompmath:masterfrom
Parcly-Taxel:bern1
Closed

Change bernoulli(1) and bernfrac(1) to +½#639
Parcly-Taxel wants to merge 10 commits intompmath:masterfrom
Parcly-Taxel:bern1

Conversation

@Parcly-Taxel
Copy link
Copy Markdown
Contributor

This pull request is part of the project at sympy/sympy#23926 and also fixes #604.

This commit is part of the project at
sympy/sympy#23926
and also fixes
mpmath#604
Copy link
Copy Markdown
Collaborator

@skirpichev skirpichev left a comment

Choose a reason for hiding this comment

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

I also would prefer a better commit message (describe reasons to change, etc).

By accepting this patch, we will be in sync with the SymPy. But most of mathematical software still uses the current mpmath's convention (e.g. Mathematica's BernoulliB or scipy.special.bernoulli). @fredrik-johansson, what do you think?

@skirpichev skirpichev removed their assignment Mar 26, 2023
@Parcly-Taxel Parcly-Taxel changed the title Change bernoulli(1) to +1/2 Change bernoulli(1) and bernfrac(1) to +½ Mar 26, 2023
Copy link
Copy Markdown
Collaborator

@skirpichev skirpichev left a comment

Choose a reason for hiding this comment

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

Ok, this seems to be correct. Of course, docstrings could be better, and there are still many unrelated changes.
Lets wait for Fredrik's opinion: if this does make sense at all (I'm +0).
UPD: https://groups.google.com/g/sage-devel/c/G4sbKoibXK4/m/LmO04RclCAAJ

Copy link
Copy Markdown
Collaborator

@skirpichev skirpichev left a comment

Choose a reason for hiding this comment

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

After reading flintlib/flint#1179 and sage discussion, I think we should be more conservative.

Lets follow to the Fredrik's suggestion and introduce a keyword (plus?) for related functions (bernoulli and bernfrac). The default value should correspond to the current convention.

Please adapt code and docstrings.

@skirpichev
Copy link
Copy Markdown
Collaborator

I fixed merge conflicts, yet I'm closing this stale pr.

Feel free to reopen it if you wish, but please adapt first code as it was suggested above (in short: control new behaviour with an option).

@skirpichev skirpichev closed this Jun 3, 2023
skirpichev pushed a commit to skirpichev/mpmath that referenced this pull request Oct 7, 2023
This commit is part of the project at
sympy/sympy#23926

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

Adapted from mpmath#639
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ellipe fails with pi as argument

2 participants