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

Add Math constants to Python #1677

Merged
merged 7 commits into from
Apr 16, 2024
Merged

Add Math constants to Python #1677

merged 7 commits into from
Apr 16, 2024

Conversation

JJtan2002
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Apr 15, 2024

Pull Request Test Coverage Report for Build 8699502394

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.003%) to 82.064%

Totals Coverage Status
Change from base Build 8690849440: 0.003%
Covered Lines: 10897
Relevant Lines: 12931

💛 - Coveralls

@RichDom2185
Copy link
Member

LGTM, thanks!

Copy link
Member

@martin-henz martin-henz left a comment

Choose a reason for hiding this comment

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

Let's match Python's math library. The constant log10e is not included there. Let's exactly match Python's math library, just like what we do for JavaScript.

(base) henz@adminsocs-MacBook-Pro-2 js-slang % python             
Python 3.10.12 (main, Aug  8 2023, 12:44:36) [Clang 14.0.3 (clang-1403.0.22.14.1)] on darwin
>>> from math import (pi)
>>> pi
3.141592653589793
>>> from math import (log10e)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: cannot import name 'log10e' from 'math' (/Users/henz/.pyenv/versions/3.10.12/lib/python3.10/lib-dynload/math.cpython-310-darwin.so)
>>> 

@@ -9,7 +9,9 @@ \subsection*{MATH Library}
\texttt{Math} library, see\\
\href{https://www.ecma-international.org/ecma-262/9.0/index.html\#sec-math-object}{\color{DarkBlue}ECMAScript Specification, Section 20.2}. Examples:
Copy link
Member

Choose a reason for hiding this comment

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

Refer to Python documentation here, not to ECMAScript. Make sure that all Python math functions and only the Python math functions are available as math_...

\texttt{Math} library, see\\
\href{https://www.ecma-international.org/ecma-262/9.0/index.html\#sec-math-object}{\color{DarkBlue}ECMAScript Specification, Section 20.2}. Examples:
\href{https://docs.python.org/3.7/library/math.html}{\color{DarkBlue}Python 3.7.17 Documentation}. Examples:
Copy link
Member

Choose a reason for hiding this comment

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

Don't you want to go for the latest stable release? https://docs.python.org/3/library/math.html

Are you implementing all functions in the Python math library? How about math.isnan, or math.factorial. If any functions are missing, please create an issue where you list what needs to be done to make the implementation consistent with the specs.

Copy link
Member

@martin-henz martin-henz left a comment

Choose a reason for hiding this comment

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

LGTM

@martin-henz martin-henz merged commit 294be20 into master Apr 16, 2024
4 checks passed
@martin-henz martin-henz deleted the pythonmathnames branch April 16, 2024 03:57
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.

None yet

4 participants