Type hints via PYI files?

[nachomaiz]

Over time I've been building PYI type stubs for the functions & metadata objects I use the most from pyreadstat.

Would it be helpful to upstream them into the repo? Let me know, happy to open a PR with suggestions. 😄

Thank you for one of the most helpful libraries in my arsenal!

[ofajardo]

sounds interesting! I am open to it, but let's wait for the file object PR to be ready first, as I guess it would change the hints ...

[nachomaiz]

Sounds good! I'm also looking forward to the support for file objects. I'll hold off until it lands. Thanks! 😁

[ofajardo]

btw, I was researching a little bit if it is possible to add the annotations directly in the pyx file, and as far as I understood mypy for instance can only use information from py or pyi files and not pyx or compiled code, is my understanding correct or do you know if somehow the annotatations could go directly in the cython code?

[nachomaiz]

Yeah that's my understanding as well.

Annoyingly most language servers also don't recognize docstrings from cython (though they do show up through inspect.signature) so I sometimes end up copying over the docstrings too, maybe setting up a script to do so on release.

Usually I go for combining the above with setting up cython to also include .pyx and .pxd files in the build so the full code is available, and hopefully some day type checkers will support them too. I think the latter also helps when cimporting.

[ofajardo]

Hmm I was not aware of the docstrings issue. Another alternative would be to transform pyreadstat.pyx to a .py file, I guess then we would have annotations and docstrings. If the performance would not be much affected, I think it could be a good solution.

[ofajardo]

Now that the reading from file handles is ready, I will make the experiment to transform pyreadstat.pyx to pyreadstat.py. If no major performance disadvantage that can be a good solution. @nachomaiz you can do the annotations directly on pyreadstat.py if you like.

The only thing that will be lost, and I kind of would miss, is that right now because it is a pyx if you introduce an argument with the wrong type, that will immediately raise an error. With a py file, of course that would not happen. What is your opinion on that?

[nachomaiz]

From what I understand there would be the option of having type checks if the Python file is compiled (which I think it would need to be given it cimports other functions and type definitions), but depending on the implementation it won't always be optimized to C types.

The options available to create a .py file that still needs to be compiled (in order to interact with readstat, etc.) would be:

def test(a: str, b: bool, c: int) -> object: ...
def test(a: str, b: cython.bint, c: cython.int) -> object: ...

Both should check types (not sure about str though) but only the latter generates C types and C code. The first one seems to use PyNumber_... and other Python-level APIs so there would probably be some performance losses there.

For complex union types like the file-path/object the compiler treats it as a Python object anyway.

The problem with the latter is that most type checkers probably would not recognize the cython types (unless cython is installed) which would be a drawback for moving towards .py files for easier type hints.

So I agree with seeing how the performance changes. If it's only at the function boundaries and the types are consistently casted to C types it probably would be fine.

But I still think .pyi files would be the easiest and most flexible option, that would allow for both performance and type accuracy.

Keen to hear your thoughts and see how things go.

[nachomaiz]

Another relatively simple change to improve the visibility of types and docstrings could be to include the .pyx files in the compiled wheels. That way it's easier to reference anything, and I think it might even allow Python to show relevant code in tracebacks.

[ofajardo]

OK I did the refactoring in the branch pyfile_dev, where now pyreadstat.py is a pure python file instead of a cython pyx one. I think this is going to be the simplest. Performance does not seem to be (significantly) affected, and when I refactored I had to create a new function in the parser and writing modules that now do the static type checking for free as it was before.

@nachomaiz Would you like and would you be able to take a look and test to see if now you can see the docstrings in your IDE, and whatever else you would be expecting to be improved/corrected?

[nachomaiz]

Thanks for letting me know! I'll take a proper look a bit later, after work. Having taken a quick peek I don't see a reason why it shouldn't work on IDEs though. 👍

Just to confirm, does the file-like object functionality only apply to reading functions?

[nachomaiz]

Seems to be working properly, I can see docstrings and even a few types get inferred like boolean/int! (mileage may vary though)

I just realized, one thing that this won't cover, which could really use some annotations, is the metadata container. That would be one of the things that could benefit the most, since you need to read off of it like a dataclass, but you never really know what type you'll get (especially inside collections) unless you try it out.

For example, the missing_range attribute could be typed with something like this in a pyi file:

from typing import TypedDict

class MissingRange(TypedDict):
    """A dictionary to hold missing ranges for variables."""

    hi: float | int
    lo: float | int

class metadata_container:
    # ...
    missing_range: dict[str, list[float | int | str | MissingRange]]

Anyway, I can add annotations in the py file and prepare a pyi file for the container so you can review. I think that's the only other bit of the public interface that is not covered by the py file, but let me know if there's anything else that I might be missing. Admittedly, I only really use the SPSS functions.

Would you prefer for me to create a new branch off of pyfile_dev? Or to wait after merging that one?

[ofajardo]

@nachomaiz good point! I have moved the metadata container to its own py file. Does that solve the issue?

[nachomaiz]

Yes, that's a good option too. It should avoid having to create a separate pyi file for it, and I think it's good that the public interface will be easier to check against a py file that gets included in the package wheels.

So yeah, let me know if you'd like to merge this first before I take a first pass at type hints, or if it's easier that I create a branch off of pyfile_dev. Happy to do either. 🙂

[ofajardo]

Let me merge into dev, then you can do a PR against that dev branch