make ndigits more generic

[JeffBezanson]

This fixes one of the problems with StaticNumbers.jl identified in #30374.

ndigits and unsigned don't work on all Integers. This just brings back the old implementation for integers that are neither signed or unsigned. Better ideas welcome.

[StefanKarpinski]

Are testing this in some way? If not, we should define a GenericInteger <: Integer type to do so.

[rfourquet]

I don't think we have documentation on integer interfaces to make us expect that a <:Signed / <:Unsigned type will have more chances to implement ndigits than an <: Integer type like StaticNumbers (for unsigned I concede that though!).

So it feels like dispatching here on the super-type is just tailor-made for StaticNumbers to work. I guess StaticNumbers could implement ndigits easily (by delegating to the wrapped number, like it does for unsigned), but that it can't inherit from a sub-type of Integer because the signedness will depend on the type parameter; in this (hypothetical future) case, BigInt(x::StaticNumber) would become slower than necessary with this PR. On the other hand, some <:Signed type may not have ndigits implemented and would benefit from the old implementation.

But unfortunatly I don't have a better idea at hand for StaticNumbers, so won't oppose to this solution... unless it would be acceptable instead to PR StaticNumbers with something like Base.ndigits0zpb(x::StaticInteger{X}, b::Integer) where {X} = Base.ndigits0zpb(X, b) ?

[JeffBezanson]

Getting ndigits to work is not too hard; we can probably have a generic implementation for all Integers. There's a method with a TODO about this, that almost works if you just remove the special cases. The calls to unsigned are the bigger problem.

[rfourquet]

The calls to unsigned are the bigger problem.

But unsigned seems to be already implemented in this package... With the definition of ndigits0zpb I gave above, BigInt(static(123)) works (bug BigIng(static(0x2)) doesn't, because a call to sign which fails). I could make a PR to StaticNumbers to implement ndigits.

[JeffBezanson]

That doesn't seem to be the case? unsigned(static(n)) and Unsigned(static(n)) don't work for me.

Also, needing to define ndigits0zpb is not a good interface. ndigits seems like exactly the kind of function that can be written generically and shouldn't need to be defined for different types.

[rfourquet]

That doesn't seem to be the case? unsigned(static(n)) and Unsigned(static(n)) don't work for me.

Ok... it works for me thanks to this line:
convert(::Type{T}, ::Union{StaticInteger{X}, StaticNumber{X}, StaticReal{X}}) where {T<:Number, X} in "StaticNumbers.jl" line 108, but it doesn't match master (I have it installed regularly with ]add StaticNumbers on a week old Julia master).

needing to define ndigits0zpb is not a good interface. ndigits seems like exactly the kind of function that can be written generically and shouldn't need to be defined for different types.

I totally agree. I would look into tackling the "TODO" about ndigits and make it work more generally, but I'm afraid I won't have time before the 1.1 release, if it's still planned within a couple of days.

[JeffBezanson]

Aha, that explains it. I need to revise my StaticNumbers PR a bit.

[JeffBezanson]

Ok! Here's a version that addresses the issue via ndigits instead. The name of the branch no longer makes sense but whatever :)

[JeffBezanson]

Sounds good; done.

[rfourquet]

Sorry, my last comment was a mess, I think it should be here BitInteger