Skip to content

Comments

Added some type hints for function parameters#85

Open
Robolightning wants to merge 1 commit intotiagocoutinho:masterfrom
StableKite:master
Open

Added some type hints for function parameters#85
Robolightning wants to merge 1 commit intotiagocoutinho:masterfrom
StableKite:master

Conversation

@Robolightning
Copy link

Added some type hints to functions to fix static analyzer warnings

def v4l2_fourcc(a, b, c, d):
import typing

CharType: typing.TypeAlias = typing.Union[str, bytes]
Copy link
Owner

Choose a reason for hiding this comment

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

Can't it be str | bytes ?

Copy link
Author

Choose a reason for hiding this comment

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

You can also use str | bytes instead of typing.Union[str, bytes], it's up to you.

Copy link
Owner

@tiagocoutinho tiagocoutinho Feb 20, 2026

Choose a reason for hiding this comment

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

I'd prefer if you change to str | bytes. I try to avoid using typing as much as I can since python standard library plans to phase out typing module

linuxpy/io.py Outdated
bytes,
os.PathLike[str],
os.PathLike[bytes]
]
Copy link
Owner

Choose a reason for hiding this comment

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

Again here can't it be str | bytes | os.PathLike[str] ...

Copy link
Author

Choose a reason for hiding this comment

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

Can

linuxpy/io.py Outdated
binary: bool = True,
blocking: bool = False,
close_on_exec: bool = True
) -> typing.Union[io.FileIO, io.TextIOWrapper]:
Copy link
Owner

Choose a reason for hiding this comment

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

Here too io.FileIO | io.TextIOWrapper?

Copy link
Author

Choose a reason for hiding this comment

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

Can too. On my project, it's common to use Union instead of | for type annotations, so I wrote it like this. + ensures compatibility with older versions of Python

Copy link
Owner

@tiagocoutinho tiagocoutinho left a comment

Choose a reason for hiding this comment

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

Thanks for the improvements
I made some comments

@Robolightning Robolightning force-pushed the master branch 2 times, most recently from 41f1b97 to ee40917 Compare February 21, 2026 20:18
@Robolightning
Copy link
Author

Thanks for the improvements I made some comments

Fixed. Ready

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.

2 participants