-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
Reduce RealNumberQ and refactor #821
Conversation
This is not a WMA builtin so it should not be protected. Possibly we should remove this altogether and add ExactNumberQ Rubi 5 defines RealNumberQ so this is a problem.
* Add slightly better test() annotation for the return type * create BooleanType and use that * Reduce use of RealNumberQ by removing it from Range[] * Move {In,}ExactNumberQ from numbers to numerical_properties which is where other tests appear. (it is smaller than numbers). * add is_number() to mathics.eval.numbers
687badc
to
31770b2
Compare
mathics/builtin/list/constructing.py
Outdated
"Range[imin_, imax_, di_]" | ||
|
||
for arg in imin, imax, di: | ||
if not is_number(arg): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, what we need to check is if they are real numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meaning Real, integer or rational
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function was poorly (ambiguously or incorrectly) named. That has been corrected. However what it does I think is the correct thing. I compared with WMA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. The comment was about the name of the internal function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There will be more PRs to remove RealNumberQ. This is just the first. |
Supersedes #820.
As is common, the changes needed are far too great to do in one programming session. So this is the first part.