search

ga4gh / ga4gh-schemas

Models and APIs for Genomic data. RETIRED 2018-01-24
http://ga4gh.org
Apache License 2.0
214 stars 114 forks source link

Deprecate `scripts/process_schemas` #821

Open david4096 opened 7 years ago

david4096 commented 7 years ago

We had recently worked with having process schemas (which generates python bindings from the proto) as part of setup.py script. This allowed protoc to generate bindings for the proto as part of normal build processes without invoking another script.

In order to make our package more portable, we opted to include the bindings in the source tree. I would like to deprecate the call to the script directly and reinstitute process_schemas as part of the setup.py process and document this pattern.

dcolligan commented 7 years ago

Didn't we move to the current arrangement of checking in the pb2 files because we couldn't guarantee that protoc was installed on the target machine? Has something changed to obviate that concern?

david4096 commented 7 years ago

Hi @dcolligan! Yep! And we can still check in the pb2, I just would like to not have to have process_schemas as a separate script to call but document the call to python setup.py develop. This also generates the version py, which I recently got stuck looking around for since it is in the .gitignore.

We can close this issue by clearly stating the requirements for building the proto using python, and demonstrating the development flow of running python setup.py develop after modifying the schemas.

david4096 commented 7 years ago

See solution in https://github.com/ga4gh/schemas/issues/828

We can close when it is clear just how and when this code path should be hit.