magenta / note-seq

A serializable note sequence representation and utilities.
Apache License 2.0
211 stars 57 forks source link

Support music_pb2 / generator_pb2 type checking #62

Closed atsukoba closed 2 years ago

atsukoba commented 2 years ago

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[bot] commented 2 years ago

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.

cghawthorne commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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

atsukoba commented 2 years ago

@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.