BUG: Fix Series/DataFrame.rank(pct=True) with more than 2**24 rows

[jschendel]

• closes BUG: Series.rank(pct=True).max() != 1 for a large series of floats  #18271
• tests added / passed
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[pep8speaks]

Hello @jschendel! Thanks for submitting the PR.

• There are no PEP8 issues in the file pandas/tests/frame/test_rank.py !

• There are no PEP8 issues in the file pandas/tests/series/test_rank.py !

• There are no PEP8 issues in the file pandas/tests/test_algos.py !

[WillAyd]

FWIW good to have this typing consistent, though I'm surprised it's required given the Cython docs say that a float gets mapped to a double:

http://docs.cython.org/en/latest/src/tutorial/caveats.html

@jbrockmendel any insights? Does that seem like a bug in Cython?

[codecov]

Codecov Report

Merging #23688 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #23688   +/-   ##
=======================================
  Coverage   92.24%   92.24%           
=======================================
  Files         161      161           
  Lines       51318    51318           
=======================================
  Hits        47339    47339           
  Misses       3979     3979

Flag	Coverage Δ
#multiple	90.63% <ø> (ø)	⬆️
#single	42.31% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a197837...a6abe28. Read the comment docs.

[jreback]

thanks!

[jbrockmendel]

though I'm surprised it's required given the Cython docs say that a float gets mapped to a double

@WillAyd no idea; we recently changed all usages of "double" and "double_t" to "float64_t" largely so I didn't have to keep double-checking that they mean the same thing. Maybe @scoder can offer some insight?

[scoder]

though I'm surprised it's required given the Cython docs say that a float gets mapped to a double

Note how that page says "Python's float type", not "C's float type". Whenever you say "cdef" in Cython, what follows is a C declaration. We specifically document that Python types like "int" and "float" are not (directly) usable in type declarations because they are shadowed by the more useful C types with the same name. There is no advantage at all in declaring a variable with those Python types, but there is good value in doing that with the C types.

In short, if you want a specific C type, name it.

[scoder]

we recently changed all usages of "double" and "double_t" to "float64_t" largely so I didn't have to keep double-checking that they mean the same thing

In theory, they do not. The C standard does not guarantee specific behaviour for the double type, only that the precision is at least twice as high as for the single precision "float" type, i.e. it gives you minimum precision guarantees.

However, as long as your code is compiled in an IEEE-754 floating point environment, which applies to pretty much all relevant system types these days, the behaviour will be exactly that of a 64-bit IEEE-754 double precision binary floating point number.

Now, in practice the float64_t type is an alias for double for exactly these reasons, which means that while it might look more exact in code, it can also suggest a false safety. For example, enabling -ffast-math in gcc will allow it to diverge from the IEEE-754 standard for optimisation purposes, and float64_t will probably not save you from that. But at least it could, in the sense that float64_t, not being a standard C type, might simply be undefined (and your code would then fail to compile) if strict 64-bit IEEE-754 semantics are not available. (Although, again strictly speaking, float64_t does not enforce IEEE-754 compliance, so a hypothetical non-IEEE-754 64-bit floating point type could still qualify. ¯_(ツ)_/¯ )

Anyway, explicit is better than implicit. If you want exact 64-bit float semantics, saying so in your code is probably a good idea. If you can live with C double semantics, and that is what CPython does internally, for example, saying so is probably also a good idea.

[jbrockmendel]

@scoder thanks for clarifying. Have I mentioned recently how nice it is to not have to worry about these things in python and 90+% of the time in cython?

@chris-b1 @jreback this is above my pay grade. Is there any chance that we need to revert parts of #23583 to use double/double_t instead of float64_t?

[scoder]

I might not have enough insight into the details here, but I don't think reverting is necessary. It's a bit more verbose that way, but it probably also reflects what your code does. Specifically, if you are programming against NumPy, then NumPy's data type API is more relevant than CPython's internals or C's double type here, so matching np.float64 in Python with float64_t in C/Cython seems right.