Skip to content

Conversation

@penguinland
Copy link
Contributor

@penguinland penguinland commented Feb 11, 2025

The (present-but-empty) py.typed file indicates to mypy (and presumably other type checkers) that this module supports type annotations.

The problem: I have a repo which imports this SDK, but running mypy main.py on it gives a bunch of errors like this:

src/config/attribute.py:3: error: Skipping analyzing "viam.proto.app.robot": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/config/config.py:1: error: Skipping analyzing "viam.proto.app.robot": module is installed, but missing library stubs or py.typed marker  [import-untyped]

These errors occur despite all the type annotations in the SDK.

The solution: in my virtual environment, I ran touch venv/lib/python3.12/site-packages/viam/py.typed, and all those errors went away. This works because of PEP561, which says "Package maintainers who wish to support type checking of their code MUST add a marker file named py.typed to their package"

This PR hopefully does the same thing to the official version. However, I'm unsure how to test this out, and would welcome help on that front!

Disclaimer: I'm still pinned to SDK version 0.29.3 from October. It's possible this has somehow been fixed since then, but if so, I didn't see it in the SDK repo.

Copy link
Member

@njooma njooma left a comment

Choose a reason for hiding this comment

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

Interesting... viam.proto.app.robot should not be typechecked IMO because it's a generated file. although it doesn't look like you're using those protos... this is interesting. i don't see any issue with adding it

@penguinland
Copy link
Contributor Author

Hm... I didn't actually look too closely at what this code has been importing. You're right that we're importing ServiceConfig, which appears to be a generated class. However, it's got stubs in src/viam/gen/app/v1/robot_pb2.pyi, which I think is what mypy will use (src/viam/proto/app/robot.py is mainly a wrapper around ...gen.app.v1.robot_pb2).

I'm gonna think about this overnight, and merge tomorrow if it still seems like a good idea...

@penguinland
Copy link
Contributor Author

...I still think this is a good idea, regardless of whether the classes I'm using are generated.

@penguinland penguinland merged commit b092a73 into viamrobotics:main Feb 12, 2025
12 checks passed
@penguinland penguinland deleted the py_typed_file branch February 12, 2025 17:00
penguinland added a commit to viam-modules/re-id-object-tracking that referenced this pull request Feb 26, 2025
I started looking at this due to the obvious errors in the type annotations mentioned in #23. 

When I started this branch, running `mypy main.py` gave 150ish errors. Between this PR and viamrobotics/viam-python-sdk#846, it's down to 90ish errors. So, there's more to do, but this is at least a start. When there are no errors, we should add another CI check that running mypy (or any other type checker, if you want something else) has no errors.


* add mypy to requirements.txt

* add Optional type annotation to all variables that can be initialized to None

* don't pass in floats when we expect ints

* fix copypasta: default value of a boolean should be a boolean, not a string

* add type annotations for tabulate module to requirements.txt

* don't return an exception, raise it

* clarify types of optional fields

* fix type annotation Image -> Optional[Image.Image]

* fix mistake from rebase conflict
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