Skip to content

Support music_pb2 / generator_pb2 type checking#62

Closed
atsukoba wants to merge 2 commits intomagenta:mainfrom
atsukoba:main
Closed

Support music_pb2 / generator_pb2 type checking#62
atsukoba wants to merge 2 commits intomagenta:mainfrom
atsukoba:main

Conversation

@atsukoba
Copy link
Copy Markdown

@atsukoba atsukoba commented May 25, 2022

Generated python files had no type information so users who use modules under note_seq.protobuf with type checker like mypy need to add lots of comments to make lines be ignored by type checkers. I added pyi stub files without changing module codes with several protobuf packages listed in /note_seq/protobuf/README.md. It should helps users use NoteSequence more comfortably and make full use of NoteSequence functions and representations in practical and creative music application!

mypy-protobuf is not tested with protobuf==3.6.1 but it worked. I confirmed some type checker outputs with offiial sample snippet like below.

Before

image

After

image

@google-cla
Copy link
Copy Markdown

google-cla bot commented May 25, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@atsukoba atsukoba changed the title Support music_pb2 / generator_pb2 type hinting Support music_pb2 / generator_pb2 type checking May 25, 2022
@cghawthorne
Copy link
Copy Markdown
Contributor

Thank you for pointing this out! I just noticed that the latest version of protoc natively supports pyi outputs, so I think we'll prefer to stick with that. I'm working on a PR for it and should have it committed soon: #63

@cghawthorne
Copy link
Copy Markdown
Contributor

@atsukoba I release a new pip package of note-seq, but I think the type hints weren't included in the new package. Can you verify if the hints are working for you, and if not, would you mind looking into what modifications are needed to setup.py to include them?

@atsukoba
Copy link
Copy Markdown
Author

atsukoba commented Jul 1, 2022

@cghawthorne Thank you for your commit and release. I confirmed that the new release (0.0.4) package doesn't contain stub files. According to mypy docs, we should add the argment package_data: Mapping[str, list[str]] to setuptools.setup function to distribute type information.

for example, we can add the arg below to track stub files in the package.

# setup.py

setup(
  name='note-seq',
  ...,
  package_data={"note_seq": ["*.pyi", "**/*.pyi"]}
)

How about trying that?

@cghawthorne
Copy link
Copy Markdown
Contributor

Thanks! Could you try v0.0.5 of the pip package? Hopefully that solves it!

@atsukoba
Copy link
Copy Markdown
Author

atsukoba commented Jul 2, 2022

@cghawthorne Thank you for updating pip package. I checked v0.0.5 and found stub pyi files for generated modules. Type checkers on my local environmen works well with the new release.

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