pypdfium2-team / pypdfium2

Python bindings to PDFium
https://pypdfium2.readthedocs.io/
425 stars 17 forks source link

chore: updates ints pdfpage.render to floats #329

Closed BobSlim closed 1 month ago

BobSlim commented 1 month ago

Important: External contributors are required to comply with the template.

Description

minimal change that improves typing and DX

Checklist

Helpers - [x] This PR changes helpers code. - The change comes with sufficient test cases to confirm correct functionality. - Any new test files are under a suitable license and have been registered in `reuse/dep5`.
Setup - This PR changes setup code (`setupsrc/`, `setup.py` etc.). - I have read through relevant doc sections [Installation -> From source][1] and [Setup Magic][2]. - I have tested the change does not break internal callers. - I believe the change is maintainable and does not cause unreasonable complexity or code pollution. - The change does not try to shift a maintenance burden or upstream downstream tasks. *Keep handlers generic, avoid overly downstream-specific or (for us) effectively dead code passages.* - I have assessed that the targeted use case cannot reasonably be satisfied by existing means, such as [Caller-provided data files][3], or the change forms a notable improvement over possible alternatives. - I believe the targeted use case is supported by pypdfium2-team. *Note that we may not want to support esoteric or artificially restricted setup envs.* [1]: https://github.com/pypdfium2-team/pypdfium2?tab=readme-ov-file#install-source [2]: https://github.com/pypdfium2-team/pypdfium2?tab=readme-ov-file#setup-magic [3]: https://github.com/pypdfium2-team/pypdfium2?tab=readme-ov-file#install-source-caller
Other - This PR changes other things, namely: ...
mara004 commented 1 month ago

I don't see what's supposed to be the benefit of this. In Python, there is no need to do this.

BobSlim commented 1 month ago

Hi @mara004, usually in my python projects I rely heavily on my type hints and IDE, especially when interfacing with libraries.

In the project I was working on, I wasted some time figuring out the cause of one of my bugs, which was caused by the mistaken type of these parameters.

Simply altering 0 -> 0.0 is a non intrusive way to improve the experience of some consumers of this library.

Increasingly, I'm seeing stronger and better features for types in python, and I find it a far more pleasant experience to lean into them. I would be happy if library authors were so inclined.

Please reconsider!

mara004 commented 1 month ago

No. As long as the rest of the library does not use type hints, there is really no point adding needless .0s. I'm not aware of any official guidelines that would recommend doing this.

To properly use our APIs you will have to read the documentation, which describes the parameters their types. A .0 indicating float for scale will not help much, you'll still need to know what it means.