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

Add stub file #108

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add stub file #108

wants to merge 2 commits into from

Conversation

Viicos
Copy link

@Viicos Viicos commented Apr 14, 2024

Fixes #105.

I'm not sure if the .pyi file needs to be explicitly included for setuptools?

@QuentinN42
Copy link

I'm also looking forward for type hints.

Is extra work needed here, if yes, I can help.

@Viicos
Copy link
Author

Viicos commented May 27, 2024

@mwilliamson, sorry for the ping. If you prefer to not have them included in the repository itself, I'll open a PR on typeshed instead :)

@mwilliamson
Copy link
Owner

I like the idea in principle of having stub files, but I think it'd be good to make sure they're kept in sync with the actual code, for instance by:

  • generating the stub files from the .pyx source
  • running a type checker over the tests
  • adding examples that should fail type checking

I'm not sure what other Cython projects typically do?

If the stub is maintained by hand, then I think it's probably worth hiding internal details such as _JqStatePool, and possibly leaving out deprecated methods in the interest of simplicity and easier maintenance.

Just to be clear: the above is meant as thoughts for discussion, rather than a request for changes in this pull request!

@Viicos
Copy link
Author

Viicos commented Jun 3, 2024

  • generating the stub files from the .pyx source

I'm not aware of such a thing. It is possible to do so for plain Python code using stubgen/stubtest. I'll try to ask on typeshed to see if someone knows about such a solution.

  • running a type checker over the tests

Sounds like a good idea.

  • adding examples that should fail type checking

We can additionally add tests using assert_type. The way it is done on typeshed to assert errors is to explicitly use a type: ignore comment and configure the type checker to report unused ignore comments.

If the stub is maintained by hand, then I think it's probably worth hiding internal details such as _JqStatePool, and possibly leaving out deprecated methods in the interest of simplicity and easier maintenance.

I also wanted to avoid having them exported, but I also kind of need them mostly for return values. I guess the thing is people should only use them "implicitly":

from jq import compile

prog = compile("...")
# prog is of type "_Program", but the user does not need to know about it.

prog.input({...})  # is of type "_ProgramWithInput, but the user also does not need to know about it.

@mwilliamson
Copy link
Owner

If the stub is maintained by hand, then I think it's probably worth hiding internal details such as _JqStatePool, and possibly leaving out deprecated methods in the interest of simplicity and easier maintenance.

I also wanted to avoid having them exported, but I also kind of need them mostly for return values. I guess the thing is people should only use them "implicitly":

from jq import compile

prog = compile("...")
# prog is of type "_Program", but the user does not need to know about it.

prog.input({...})  # is of type "_ProgramWithInput, but the user also does not need to know about it.

I think it's fine (and indeed, necessary) to expose, say, _ProgramWithInput, but I was thinking more about things like _JqStatePool and _ProgramWithInput.__init__.

@Viicos
Copy link
Author

Viicos commented Jun 30, 2024

@mwilliamson regarding tests, do you want me to create a new tox env, or run everything in the default one?

@mwilliamson
Copy link
Owner

I think having the tests for the types run in the same envs as the existing tests is simplest, unless there's a particular downside to doing so?

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

Successfully merging this pull request may close these issues.

Add type hints as a .pyi stub file
3 participants