Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Guide] Use location data services #1247

Merged
merged 22 commits into from
Nov 28, 2019

Conversation

elenatorro
Copy link
Contributor

@elenatorro elenatorro commented Nov 26, 2019

Related issue: #995

First review:

  • Notebook: examples/data_services/starbucks_stores.ipynb

Notes: The isolines service can return some empty geometries. I've added the code needed to delete those geometries, but I'd like to know how to explain this situation in the guide.

Second review:

  • Make exclusive True by default, and remove the other example
  • Change the visualization of the precision using bins and / or size_continuous_helper
  • Find a solution / workaround for the source_id issue
  • [ ] Ask about the empty geometries and check a better way to filter them, and explain why it might happen
  • Add the descriptions (the guide itself)
  • Write the conclusion
  • Publish examples and generate the markdown guide (TBD)

Others To Do:

  • Set by default the table privacy to 'private' when using cached table
  • Allow custom popups (minimum change needed for the examples)
  • In the Geocoding service, change the cached parameter to boolean, and add a table_name parameter to be aligned with Isolines service

@alasarr
Copy link
Contributor

alasarr commented Nov 26, 2019

  • More descriptions

I see pending of descriptions in the guide: Explain other settings available, using isodistances this time. The idea of the document is to be 100% document, the content should be quite similar to the test of the final guide.

  • source field isochrones' table

image

There is no way to get the reference between the isochrones and the original dataset. Talk with @Jesus89 about this issue, or we fix it or we document the temporal work around

  • Exclusive parameter

We've mixed geometries Multipolygon and Polygon when using exclusive=True.

image

Two options:

  1. Don't document this parameter at the example and add an issue to improve it. Visualize just with rings.
  2. Fix the issue. If it takes less than 1h
  • Empty geometries
    Why do we have empty geometries?

If so explain why (but would be better in the doc to avoid this), and let's change the way we filter to something more Pandas friendly.

isodistances_cdf = isodistances_cdf[isodistances_cdf['geometry'].map(lambda geometry: not geometry.is_empty)]
isodistances_cdf = isodistances_cdf[isodistances_cdf['geometry'].notnull()]

Copy link
Contributor

@alasarr alasarr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to fix the previous comments

@alasarr
Copy link
Contributor

alasarr commented Nov 26, 2019

Regarding the credentials, let's remove the inline definition of the API KEY in favor of the credentials file

@elenatorro
Copy link
Contributor Author

elenatorro commented Nov 26, 2019

Regarding the feedback, I'm going to follow these steps:

  • Make exclusive True by default, and remove the other example
  • Change the visualization of the precision using bins and / or size_continuous_helper
  • Find a solution / workaround for the source_id issue
  • Add the descriptions (the guide itself)
  • Write the conclusion

@elenatorro elenatorro closed this Nov 27, 2019
@elenatorro elenatorro reopened this Nov 27, 2019
@elenatorro
Copy link
Contributor Author

This is ready for a second review, I've attached the notebook in html as well in the issue cc @alasarr @cmongut @Jesus89

Copy link
Contributor

@cmongut cmongut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 Awesome Elena!

About geocoding intro: I would show a simple geocoding and its visualization before talking about quotas.

About geocoding quotas: You tell me that I have to keep the the_geom and carto_geocode_hash columns but you don't show me how to do it. Maybe this is more a implementation detail but I would only talk about the cached parameter in terms of cache.

About isochrones and isodistances: I suggest the same as with geocoding, show how to do it as simple as possible and visualize it. Then talk about quota.

About where to place a new store: it's pretty cool. I would create another section for it and make clear this is just a simplified example because finding a new store location is not easy.

@elenatorro
Copy link
Contributor Author

Thanks for the review! A few comments:

  • Quota: we've discussed previously to introduce the concept of quota and use the dry_run before visualizing / using the services because we wanted to prevent the user. As we've talked about this many times and I thought it was clear, I just want to double check that to avoid confusion.

  • About the new store: This 'new section' can be the Using all together section, right? I know deciding where to place a new store is complicated and it's not something we should tackle in this guide. The purpose of the 'new place' is to see a different use of using the geocoding results and several layers with isolines together. Looking for an optimal store is a deep and advanced analysis, so I wouldn't extend it more. In the future, if we've an advanced guide for this, we can link it 👌

@elenatorro elenatorro requested a review from Jesus89 November 27, 2019 12:04
cartoframes/io/carto.py Outdated Show resolved Hide resolved
@elenatorro elenatorro changed the title Use location data services [Guide] Use location data services Nov 27, 2019
Copy link
Contributor

@alasarr alasarr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Standarize the usage of credentials set_default_credentials(‘creds.json’)

  • Quite technical description for isolines. We're addressing users with a non-deep knowledge on geospatial stuff. Let's keep the technical stuff since it's not the quickstart but let's move it down. Just move the following text at the beginning :In this guide we're using the Isochrones to know the walking area by time for each Starbucks store, and the Isodistances to discover the walking area by distance.

  • Quota consumption and cache not clear for me

We're missing in the examples the case where the user has a dataframe with the carto_geocode_hash column. I'd do:
a) example without cache. Outside of quota consumption at previous point.
b) Client-side cache (explain there what is the hash)
c) Server-side cache (fully managed but we're storing there data)

@elenatorro elenatorro merged commit 7deed1f into develop Nov 28, 2019
@elenatorro elenatorro deleted the guides/use-location-data-services branch November 28, 2019 09:29
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.

4 participants