Skip to content
This repository was archived by the owner on Aug 2, 2022. It is now read-only.

Conversation

@critical-path
Copy link
Contributor

This attempts to address #228. I would be happy to make changes to the code, tests, etc. Thank you!

docs/maps.md Outdated
Call the `get_tile` method, passing in values for `map_id`, `z` (zoom), `x` (column), and `y` (row). Pass in values for optional arguments as necessary - `retina` (double scale), `file_format`, `style_id`, and `timestamp`.

```python
>>> response = maps.get_tile("mapbox.streets", z=0, x=0, y=0)
Copy link
Contributor

Choose a reason for hiding this comment

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

@critical-path @perrygeo I think I'd prefer the tile params to be positional arguments in x, y, z order so we can pass in mercantile tiles like

get_tile(mapid, *Tile(0, 0, 0))

"get" is implied in the method, so how about just tile() ?

docs/maps.md Outdated
Call the `get_html_slippy_map` method, passing in a value for `map_id`. Pass in values for optional arguments as necessary - `options` (map controls and behaviors), `z`, `lat`, and `lon`.

```python
>>> response = maps.get_html_slippy_map("mapbox.streets")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not certain about the usefulness of getting a slippy map in Python. I think we could eliminate this method and have less API to support.

```

Call the `get_vector_features` method, passing in a value for `map_id`. Pass in a value for the optional argument, `feature_format`, as necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we could call this features() and only support GeoJSON-like features. Thoughts @perrygeo ?

docs/maps.md Outdated
Call the `get_tilejson_metadata` method, passing in a value for `map_id`. Pass in a value for the optional argument, `secure`, as necessary.

```python
>>> response = maps.get_tilejson_metadata("mapbox.streets")
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just metadata() ?

@critical-path
Copy link
Contributor Author

critical-path commented May 26, 2018

I have made all but one of the requested changes. I will wait for confirmation before removing the option to request data in either GeoJSON or KML format via the features method. Thank you!

@perrygeo
Copy link
Contributor

I'm inclined to keep the KML format option. I don't imagine too many people will use it but it's consistent with our design goal of keeping the mapbox-sdk-py as an un-opinionated, lightweight wrapper over the HTTP requests.

Copy link
Contributor

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

@critical-path this is very good work. Thank you. Would you be willing to rename standalone_marker to marker ? That's the last change I'd like to request.

I'm running through the code examples in maps.md and I'll need to make a few changes to get them to pass as doctests, but am quite happy to do that.

@critical-path
Copy link
Contributor Author

I have replaced all instances of "standalone_marker" with "marker" in the code, tests, and docs. Thank you!

@sgillies sgillies added this to the 0.17 milestone May 29, 2018
@sgillies sgillies merged commit 037a53c into mapbox:master May 29, 2018
@perrygeo perrygeo mentioned this pull request May 30, 2018
@sgillies sgillies mentioned this pull request May 30, 2018
@critical-path critical-path deleted the maps-api branch September 25, 2018 00:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants