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

Improve API client UX #35

Closed thekaveman closed 5 years ago

thekaveman commented 5 years ago

Right now the usage is a multi-step process including at least:

  1. get the providers registry
  2. filter registry for desired providers
  3. configure resulting providers with auth_token etc., assuming you know how to generate all the different types of tokens
  4. finally, use a ProviderClient to get_status_changes() or get_trips()

It is also somewhat confusing to have a ProviderClient that "manages" or queries multiple Providers at once. The code is probably over-complicated because of this as well...

This could all be simplified with some specific improvements to this class:

johnclary commented 5 years ago

another use case: i would like to use the API client modules but am not interested in the db loading. from that perspective it may be logical to split the api/db purposes into separate packages.

i'll be spending more time with this project in the coming weeks. hope i can contribute.

johnclary commented 5 years ago

consider removing support for "managing" multiple providers at once - one Provider, one ProviderClient

+1

thekaveman commented 5 years ago

another use case: i would like to use the API client modules but am not interested in the db loading. from that perspective it may be logical to split the api/db purposes into separate packages

I'm not sure I see the benefit here. The only dependency related to mds.db that isn't also used elsewhere is sqlalchemy; there isn't much to be gained in that regard from separating api and db.

Of course you don't have to import mds.db! Is there another concern that I'm not thinking of?

johnclary commented 5 years ago

True. I'm mostly thinking of the pip install experience. My install happens to be choking on geopandas, but I haven't spent any time on it yet (and I'd be surprised if that's hard to resolve). Definitely less of a concern if the dependencies are shared.

johnclary commented 5 years ago

as an aside, looks like geopandas dependency can easily be dropped. will open new issue/pr.

thekaveman commented 5 years ago

as an aside, looks like geopandas dependency can easily be dropped. will open new issue/pr.

It's only used here: https://github.com/CityofSantaMonica/mds-provider/blob/dc5b7e0b2d74a89fbcbd2b923548f7378e567eb3/mds/json.py#L70

To convert a shapely.geometry.Shape into a GeoJSON object. Probably extreme overkill to use an entire library just for that 😆 Happy to accept a PR to remove that dependency.

ezheidtmann commented 5 years ago

The work I'm doing in #46 and #45 is moving towards "one provider -> one ProviderClient". This seems to be independent of the two other items listed in this issue (provider names & provider config).

thekaveman commented 5 years ago

Thanks to #40 and #71 the above changes will be made, including the move to a single provider per ProviderClient.

Initialize with a provider name or identifier directly:

client_A = ProviderClient("ProviderA")
client_B = ProviderClient("da80482e-1118-48e8-bccc-38492960bd17")

# optionally pass in configuration data
client_C = ProviderClient("ProviderC", config={ "token": "private" })

Use a provider name or identifier at request time:

client = ProviderClient()

trips_A = client.get_trips("ProviderA")
trips_B = client.get_trips("da80482e-1118-48e8-bccc-38492960bd17")

# optionally pass in configuration data
trips_C = client.get_trips("ProviderC", config={ "token": "private" })