-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add stubs for JACK-Client #4809
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 stubs for JACK-Client #4809
Conversation
third_party/3/jack/__init__.pyi
Outdated
|
||
JackPositionT = Any | ||
|
||
class CBufferType: |
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.
This only exists because I couldn't for the life of me find the type definition for cffi's buffer
type; if someone can think of a better way of doing this I'd appreciate it!
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.
See perhaps python/typing#593 (summary: there's no good way yet).
fffbc1d
to
0f4913b
Compare
third_party/3/jack/__init__.pyi
Outdated
|
||
NDArray = Any # FIXME: no typings for numpy arrays | ||
|
||
JackPositionT = Any |
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.
This is a raw C type (https://github.com/spatialaudio/jackclient-python/blob/master/src/jack.py#L220) and I'm not sure how to type it correctly so I left it as Any
.
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.
Maybe type it as a private class (class _JackPositionT: ...
).
Also, all types that don't exist at runtime should be prefixed with underscores.
0f4913b
to
ed51499
Compare
Looks (to my untrained eye) like the mypy failure is unrelated (errors from |
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.
I am not familiar with this library, but spotchecked a few functions and they look good. I left a few minor comments.
third_party/3/jack/__init__.pyi
Outdated
|
||
NDArray = Any # FIXME: no typings for numpy arrays | ||
|
||
JackPositionT = Any |
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.
Maybe type it as a private class (class _JackPositionT: ...
).
Also, all types that don't exist at runtime should be prefixed with underscores.
third_party/3/jack/__init__.pyi
Outdated
|
||
JackPositionT = Any | ||
|
||
class CBufferType: |
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.
See perhaps python/typing#593 (summary: there's no good way yet).
|
||
class CBufferType: | ||
@overload | ||
def __getitem__(self, key: int) -> str: ... |
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.
The str/bytes difference here looks odd. Maybe we need cffi stubs with a more precise type though.
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.
I got that from the cffi docs:
buf[:]
orbytes(buf)
: copy data out of the buffer, returning a regular byte string (or buf[start:end] for a part)
buf[:] = newstr
: copy data into the buffer (or buf[start:end] = newstr)
len(buf)
,buf[index]
,buf[index] = newchar
: access as a sequence of characters.
It's entirely possible I've misinterpreted them though
stubtest has some complaints, which look valid on spot check:
|
ed51499
to
1ddb74c
Compare
@JelleZijlstra @hauntsaninja Addressed. @hauntsaninja I added types for the constructors of the various internal classes (Port, Status, etc.) although I left a couple as |
faab5e8
to
b1d1603
Compare
b1d1603
to
3f4210c
Compare
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.
Thanks, this looks good to me!
No description provided.