CityofSantaMonica / mds-provider

Python tools for working with MDS Provider data
https://github.com/openmobilityfoundation/mobility-data-specification
MIT License
18 stars 20 forks source link

Update JSON schema URLs #86

Closed cmdoptesc closed 4 years ago

cmdoptesc commented 4 years ago

Hi @thekaveman, while trying to run docker-compose fake.... in mds-provider-services, this error popped up:

ValueError: Problem requesting schema from: https://raw.githubusercontent.com/CityOfLosAngeles/mobility-data-specification/master/provider/trips.json

This is because https://github.com/CityOfLosAngeles/mobility-data-specification/ redirects to https://github.com/openmobilityfoundation/mobility-data-specification.

As such, I've updated a few things in github.py to reflect the state of the repo.

In addition, I've be submitting another PR in mds-provider-services to fix a small thing preventing docker-compose fake... from running.

Thanks!

thekaveman commented 4 years ago

Hi @cmdoptesc, thanks for the PR. Since neither this library nor mds-provider-services officially support MDS 0.4.x yet, if you use an explicit version parameter for the 0.3.x (or 0.2.x if you prefer) line, the existing code should still work. In terms of the command you were running, that would look similar to:

docker-compose run fake --version 0.3.2 

(The error is due to the schema files moving to the sub-directory in 0.4.0, not due to the repo moving from LA to OMF).

As for the changes introduced in this PR, we have a tracking issue over at #79 which includes some of the URL adjustments you've made (thanks!). For version decisions, please make use of the mds.versions.Version object rather than raw value parsing. We use this pattern regularly throughout both this library and mds-provider-services, for example here.

cmdoptesc commented 4 years ago

The error is due to the schema files moving to the sub-directory in 0.4.0, not due to the repo moving from LA to OMF

Thanks for the correction @thekaveman.

Looking at the code in mds-provider-services, it seems like docker-compose run fake doesn't use the --version option when initiating the MDS json schema here:

schema = mds.Schema(mds.TRIPS)

https://github.com/CityofSantaMonica/mds-provider-services/blob/86254046108d5aa0f27e2dc68bcb7b4b35588d62/fake/main.py#L35

This might lead mismatches in vehicle types or propulsion types if additional types are introduced in subsequent versions of MDS.

As such, would you be open to me removing the defaults in the parser.add_argument here, and filling those in after the corresponding MDS schema has been created?

thekaveman commented 4 years ago

it seems like docker-compose run fake doesn't use the --version option when initiating the MDS json schema

Quite the chicken/egg problem isn't it!? Thanks for catching - yes, definitely open to cleaning up the CLI setup for fake. Could you also take care of vehicle_types here. Looks like the help param of each of those praser.add_argument calls references the schema as well...