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

Rename BigInteger functions to camelCase and unstablize number theory functions #22788

Merged
merged 20 commits into from
Jul 20, 2023

Conversation

jabraham17
Copy link
Member

@jabraham17 jabraham17 commented Jul 20, 2023

Implements a number of renaming of functions in the BigInteger module to match the Chapel naming style. Also marks several functions as unstable that may move in the future.

Summary of changes

  • Rename hamdist(a) to hammingDistance(x), marked as unstable
  • Rename nextprime(result, a) to nextPrime(result, x), marked as unstable
  • Rename lucnum/lucnum2 to lucNum/lucNum2, marked as unstable
  • Marked jacobi/legendre/kronecker as unstable
  • Marked gcd/lcm/fac/bin/fib/fib2/probablyPrime as unstable
  • Remove previously deprecated gcdext and probab_prime_p
    • these have been deprecated for some time, removing them now as they are not used anymore and it allows for cleaner docs

New tests

  • adds test/deprecated/BigInteger/deprecateHamDist.chpl
  • adds test/deprecated/BigInteger/deprecateNumberTheory.chpl
  • adds test/unstable/BigInteger/unstableHammingDistance.chpl
  • adds test/unstable/BigInteger/unstableNumberTheory.chpl
  • removes test/deprecated/BigInteger/deprecateGcdext.chpl
  • removes test/deprecated/BigInteger/deprecateProbabPrimeP.chpl

Testing

  • paratest with futures
  • paratest + gasnet
  • built and checked docs

[Reviewed by @bmcdonald3]

closes #17733
partially implements decisions from #17724
marks functions in #17697 as unstable
closes cray/chapel-private#5098

Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Copy link
Member

@bmcdonald3 bmcdonald3 left a comment

Choose a reason for hiding this comment

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

Just one question about the unstable warning messages.

:return: the Jacobi symbol
:rtype: ``int``
*/
@unstable("jacobi is unstable and may move in the future")
Copy link
Member

Choose a reason for hiding this comment

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

Is all that unstable means is that something may move in the future? I'm not sure we know for sure what might happen to any of these unstable methods, we just reserve the right to do whatever we want with them, so I think the wording "may move" sounds odd to me. Something like "may change" or "may be removed" or something makes more sense to me. What do you think?

Copy link
Member Author

@jabraham17 jabraham17 Jul 20, 2023

Choose a reason for hiding this comment

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

I agree and I think I prefer "may change" since we reserve the right to do whatever we want with them

Signed-off-by: Jade Abraham <[email protected]>
@jabraham17 jabraham17 merged commit d4902b2 into chapel-lang:main Jul 20, 2023
7 checks passed
@jabraham17 jabraham17 deleted the bigint-unstable-bundle branch July 20, 2023 22:57
jabraham17 added a commit that referenced this pull request Jan 11, 2024
Removes almost all of the remaining BigInteger deprecations

This PR removes the deprecations from the following PRs I authored
- #22775
- #22888/#22774
- #22818
- #22794
- #22788
- #22121

I also removed the remaining deprecations done by Yash Raj
- #18855
- #18827

I moved one test out of the deprecated/BigInteger directory,
`bigint_getlimbs.chpl`, since its not a deprecation test and actually
checks functionality.


Testing
- [x] paratest without comm
- [x] paratest with comm
- [x] built docs and checked them

[Reviewed by @ShreyasKhandekar]
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.

[Library Stabilization] Should we rename the BigInteger lucnum* methods?
2 participants