-
Notifications
You must be signed in to change notification settings - Fork 26
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 python type information for MessageQueue class #54
base: develop
Are you sure you want to change the base?
Conversation
c5a3259
to
219d9cd
Compare
Hi @sillydan1, thanks for the PR! Naïve question -- what tools exist for me to test that the info in |
I just cross-referenced the After a bit of googling, I found that you could use I've done some usage exploration and it there are still some things that are missing:
|
Sure, that makes sense, and I trust you, but it's not an automated process. This module is extremely stable, so I'm not too worried that it will require a lot of work to keep For that, I would need something that can consume
I realize this last point would require that I write some custom test code to execute functions and examine the type of the returned object, but the first two items feel like fairly straightforward introspection and comparison. I feel like every project that provides a |
I am not aware of a standalone tool that can do that, but it should be possible to write one based on the output generated from |
OK, thanks. What tool do you use to consume the info in |
a78f537
to
91e4508
Compare
91e4508
to
311bae2
Compare
Just my text editor's language server, which is using pyright, basedpyright, or mypy behind the scenes. |
I wrote a draft of what a type-test would look like. It compares the This is quite the rabbit hole and possibly not worth the effort. I think I might just use the package untyped and abandon this mini-adventure / side-quest. |
@@ -9,5 +9,5 @@ matrix: | |||
osx_image: xcode11.2 # Python 3.7 | |||
language: shell # 'language: python' is an error on Travis CI macOS | |||
|
|||
install: python setup.py install | |||
install: python setup.py install && python pip install pybind11-stubgen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I haven't used travis CI before, so I dont know if this is the correct place to do this.
I appreciate the well-intentioned PR, and I you for talking through it with me. I apologize for my lack of knowledge about Python typing, the state of tooling, best practices, etc. Here's some thinking out loud, using mypy as a proxy for all type checkers since it seems to be the most popular. I welcome your comments and thoughts. I apologize if any of this is blindingly obvious. The three criteria I mentioned above are what I'd want to satisfy in order to feel confident about the stub file.
I think mypy's stubtest feature would handle item 1. The feature has caveats, but its runtime introspection should raise an alert if it can't find something in the module that it expects to be there. Item 2 is more difficult, AFAICT. It sounds like stubgen (https://mypy.readthedocs.io/en/stable/stubgen.html#stubgen) can create the stub file, but the doc says "The auto-generated stub files often require some manual updates...". It sounds like it's up to the module owner (me) to fill in the details and get them right, which opens up a new space for bugs to occur. That might be an acceptable tradeoff, though. Item 3 can probably be covered with a mix of mypy and custom tests. I'm sure mypy can handle the simple stuff like "check that Even though I don't use typing, I understand that it's valuable to others so I'm willing to support it. I want to make sure I get it right, though. I'm generally of the mind that vague and correct is better than specific and wrong, and that certainly applies here. |
That sounds more or less correct.
In terms of asserting the function parameters and return types, I would recommend using the
I was thinking that maybe just autogenerating the stub file using either the As for me, I'll just use the module untyped, as this was a side-quest for a side-project to my side-project. I think I'll just "nip it in the bud" and move along 😄 hope that is okay |
Thanks. I looked at your unit tests, and that filled in the missing link for me. I didn't realize that the
This is fascinating to me 🙂 One of the reasons I haven't been very interested in typing in Python is precisely because they're only hints and not guaranteed to be correct. To me, the knowledge that "
That's totally fine and I appreciate all the input and effort. Please leave this pull request open, because I don't want my preferences to get in the way of a feature that other people might find useful. I'd like more time to consider it. |
Note that this only includes theMessageQueue
class as it was the only thing I needed (so far) - I would love to add the rest of the python bindings as well if this is a desired addition.Added the rest of the things mentioned in
USAGE.md
for the sake of completeness.