Skip to content

Add partial stubs for fractions #544

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

Merged
merged 2 commits into from
Sep 14, 2016

Conversation

Michael0x2a
Copy link
Contributor

@Michael0x2a Michael0x2a commented Sep 14, 2016

This commit adds some incomplete stubs for the fractions module.

In particular, this pull request is basically a less complete version of #94 -- this pull request punts on adding type signatures to functions like __add__ and just sticks with annotating the more obvious functions and methods. This isn't ideal, but does allow us to move forward with creating stubs for the statistics module.

This pull request requires #545 to first be merged and accepted. (For some reason, the stubs for decimal in Python 3 are relatively complete, but the stubs for Python 2 are not, which is causing the current test failure).

However, this pull request can be reviewed and accepted independently from my other recent pull request that refines numbers.pyi. Once that other pull request is accepted, I'm planning on following up with another pull request that adds the __hash__ function to the Fractions class.

@gvanrossum
Copy link
Member

gvanrossum commented Sep 14, 2016 via email

@Michael0x2a
Copy link
Contributor Author

Yeah -- the fix for that is here: #545

(Python 2's stubs for Decimals seem to be much less complete then Python 3's stubs, which is what's causing the incompatibility)

@gvanrossum
Copy link
Member

gvanrossum commented Sep 14, 2016 via email

@gvanrossum
Copy link
Member

Now that the improved decimal stubs have landed can you rebase and re-push? to trigger the tests?

This commit adds some incomplete stubs for the fractions module. In
particular, this commit does not add type signatures for the more
complex functions (such as `__add__`), and just leaves their types as
effectively `Any`.
@Michael0x2a
Copy link
Contributor Author

@gvanrossum -- done! It's green now :)

_TNum = TypeVar('_TNum', int, float, Decimal, Complex)

@overload
def gcd(a: _TComplex, b: _TComplex) -> _TComplex: ...
Copy link
Member

Choose a reason for hiding this comment

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

I don't know what the docs say, but the module doesn't seem to support complex arguments here.

Copy link
Member

Choose a reason for hiding this comment

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

At least not in Python 3. It does in Python 2?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, so it looks like the return type of gcd here ultimately depends on what the type of a % b is, which makes the gcd function more complicated then I was originally expecting.

Some things I observed:

  • numbers.Complex does not have any kind of dunder methods related to mods, so shouldn't be a legal input type in either Python 2 or Python 3.
  • However, the builtins complex type does allow you to take the mod in Python 2, but NOT in Python 3.
  • If one argument is a complex number, and the other is any other type, the gcd function will either throw a TypeError or will enter an infinite loop (!!).

I also found the full matrix of allowable parameters to return types is this (not taking into account whatever any custom number subclasses might do):

            | int          float        Fraction     Decimal      complex
------------+ -----------  -----------  -----------  -----------  -----------
int         | int          float        Fraction     Decimal      Inf. Loop
float       | float        float        float        TypeError    Inf. Loop
Fraction    | Fraction     float        Fraction     TypeError    TypeError
Decimal     | Decimal      TypeError    TypeError    Decimal      TypeError
complex     | Inf. Loop    Inf. Loop    TypeError    TypeError    complex

Thankfully, the diagonal (where both parameters are the same type) appear to be relatively sane.

So, given all of the above, and given that fractions.gcd is a deprecated function anyways as of Python 3 (people are supposed to use math.gcd instead, which accepts only ints), would you rather I:

  • Attempt to come up with a system of overloads to handle the full matrix.
  • Attempt to handle the case where both parameters are the same type, and just provide separate annotations for Python 2 and 3 to handle complex numbers.
  • Just disallow allowing complex numbers as parameters completely on the grounds that nobody is actually going to do that/they're going to be using cmath instead.
  • Don't bother providing a type signature, and just make the input and output types Any.
  • Something else?

@gvanrossum
Copy link
Member

Wow, you're reasoning like pytype. :-)

In 3.5, fractions.gcd() is deprecated in favor of math.gcd(), which only supports integers. I propose to make the type signature do that in all versions. Even the 2.7 docs say "integers"...

@Michael0x2a
Copy link
Contributor Author

Hmm, ok -- I can do that.

I guess the only thing I'm hesitant about is that the source code of fractions.gcd explicitly handles the case where neither parameters are ints, which actually makes it bit more general-purpose then math.gcd.

But it seems like we're not really going to care about that?

@Michael0x2a
Copy link
Contributor Author

Or actually, maybe make it just accept either int or numbers.Integral?

@gvanrossum
Copy link
Member

Looking at the C source for math.gcd() it seems that supports things with
an index method, which is a proxy for being a sort of integer. So I
think Union[int, Integral] is fine!

This commit...

1. Adds back in the __hash__ method
2. Adds a TODO comment
3. Defines a more sane interface for functions.gcd
4. Replaces `_TNum` with a more useful `_ComparableNum` union.
@gvanrossum gvanrossum merged commit ec2b9ce into python:master Sep 14, 2016
@Michael0x2a Michael0x2a deleted the add-partial-fractions branch September 14, 2016 23:41
@gvanrossum gvanrossum mentioned this pull request Sep 14, 2016
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.

2 participants