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

Improve stub for enum, error, logging, stream #1311

Merged
merged 8 commits into from
Mar 5, 2024

Conversation

laggykiller
Copy link
Contributor

No description provided.

@laggykiller laggykiller changed the title Improve stub for enum, error, logging, option, stream Improve stub for enum, error, logging, stream Mar 4, 2024
@WyattBlue
Copy link
Member

I don't know if the stubs for enum, error, logging are even accurate. It's been a while since I reviewed that code and I know logging needs an overhaul in general.

stream.pyx shouldn't be included with these files.

@laggykiller
Copy link
Contributor Author

laggykiller commented Mar 4, 2024

I don't know if the stubs for enum, error, logging are even accurate.

I made the stub by reading the pyx files, as well as running pyav, open a video and check if those attributes could be accessed.

stream.pyx shouldn't be included with these files.

Please reconsider about this. Even if the user is not supposed to create object from that class, it is easy for us to create a stub file next to the pyx that is responsible for it. Stream from stream.pyx is baseclass for VideoStream and AudioStream. If we do not create stub for stream.pyx, further changes in stream.pyx (e.g. add/remove attributes from Stream) would require us to edit av.video.stream and av.audio.stream stub files. This is not convenient for future development.

av/enum.pyi Outdated Show resolved Hide resolved
av/enum.pyi Outdated Show resolved Hide resolved
@laggykiller
Copy link
Contributor Author

Fixed, please review!

av/enum.pyi Show resolved Hide resolved
av/enum.pyi Outdated
def __reduce__(self): ...
def __eq__(self, other) -> bool: ...
def __ne__(self, other) -> bool: ...
def __reduce__(self) -> tuple[bool, ...]: ...
Copy link
Member

@WyattBlue WyattBlue Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually have no clue what __reduce__ returns, or if this method is even needed at all. Just use set the return type to Any

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be tuple[Callable[[str, str, str], Any], tuple[str, str, str]].

enum.pyx:

    def __reduce__(self):
        return (_unpickle, (self.__class__.__module__, self.__class__.__name__, self.name))
def _unpickle(mod_name, cls_name, item_name):
    mod = __import__(mod_name, fromlist=["."])
    cls = getattr(mod, cls_name)
    return cls[item_name]

Copy link
Contributor Author

@laggykiller laggykiller Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mystery solved:

>>> from av.video.frame import PictureType
>>> t = PictureType
>>> t
<class 'av.video.frame.PictureType'>
>>> t[0]
<av.video.frame.PictureType:NONE(0x0)>
>>> t[0].__reduce__()
(<cyfunction _unpickle at 0x73e2344805f0>, ('av.video.frame', 'PictureType', 'NONE'))
>>> t[0].__reduce__()[0](*t[0].__reduce__()[1])
<av.video.frame.PictureType:NONE(0x0)>
>>> type(t[0].__reduce__()[0](*t[0].__reduce__()[1]))
<class 'av.video.frame.PictureType'>

So it should be tuple[Callable[[str, str, str], EnumItem], tuple[str, str, str]]

@WyattBlue WyattBlue merged commit e4aa4c6 into PyAV-Org:main Mar 5, 2024
18 checks passed
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.

None yet

2 participants