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

Define typemin/typemax for BigInt #29815

Closed
wants to merge 1 commit into from
Closed

Define typemin/typemax for BigInt #29815

wants to merge 1 commit into from

Conversation

omus
Copy link
Member

@omus omus commented Oct 26, 2018

I think it would be nice to have typemin/typemax defined for all Integer subtypes. I don't think it unreasonable to say that any custom type which is a subtype of Integer should have these methods defined for that type.

This concept was thought of this while working on speeding up some generic code in FixedPointDecimals which used typemax on Integer types. While creating this PR I discovered hastypemax which works but I think it should be eventually be deprecated as the applicable(typemax, T) fallback is not very performant for custom types.

@omus omus force-pushed the cv/typemax-bigint branch from 4378723 to a5dd928 Compare October 26, 2018 19:18
@nalimilan
Copy link
Member

Returning nothing is rather weird here. I'd rather make applicable fast... See also #16422.

@StefanKarpinski
Copy link
Member

Returning nothing here is definitely weird. I agree with @nalimilan that making applicable faster would be much preferable. Should be possible to have zero overhead in most situations.

@simonbyrne
Copy link
Contributor

I still think it should be AlephNull...

@JeffBezanson
Copy link
Member

I agree returning nothing doesn't help much, since there is still a special case you have to check for.

@omus
Copy link
Member Author

omus commented Oct 28, 2018

If we can make applicable fast than I agree it is preferable to this solution. For now we'll have a special case in FixedPointDecimals until that time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bignums BigInt and BigFloat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants