Fix map_geometries() loss of feature IDs

[bryik]

I noticed that a Feature passed to geojson.utils.map_tuples() loses it's ID. Here is a simple replication.

The cause seems to be that IDs are not included in the dict returned by geojson.utils.map_geometry().

This PR:

• adds the ID to the dict aca22cb
• adds a test to check ID is present after mapping f3e876a

[codecov]

Codecov Report

Merging #128 into master will increase coverage by 1.78%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
+ Coverage   93.33%   95.11%   +1.78%     
==========================================
  Files          10       10              
  Lines         345      348       +3     
  Branches       69       70       +1     
==========================================
+ Hits          322      331       +9     
+ Misses         14        9       -5     
+ Partials        9        8       -1

Impacted Files	Coverage Δ
geojson/utils.py	87.96% <75%> (+6.05%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e96c0e9...20880c0. Read the comment docs.

[bryik]

I don't know what codecov/patch is or why it is failing.

[auvipy]

It's telling to improve the cov little bt more.

[rayrrr]

Hi @bryik, thank you for your contribution. However, I'm leaning against making this change, because I don't know that id should be returned by those functions.

If you read the docstrings (the main comments directly under each function declaration) in https://github.com/jazzband/python-geojson/blob/master/geojson/utils.py then you will see that the various utility functions in this repo are only intended to return partial data, not a full representation of the input object.

So, my understanding is that the utility functions are not intended to be a "put your object in one end and get it out the other transformed" functions, but "give me your object and I'll give you some transformed data you can add to it" functions.

Would using them this way meet your needs? In other words, change the second half of your example to:

geojson_feature = geojson.Feature(
    id=example["id"],
    geometry=geojson.Point(example["geometry"]["coordinates"]),
    properties=example["properties"])

# The ID is present.
print("id before map:", geojson_feature["id"])

# Map over the feature's tuples (without doing anything).
geojson_feature['mapped_geometry'] = geojson.utils.map_tuples(lambda t: t, geojson_feature)

# The ID and mapped data are now both available on the original object!
try:
    print("id:", geojson_feature["id"])
except KeyError:
    print("ID is missing!")

[bryik]

Hello @rayrrr. I'm not quite sure I understand your proposal.

geojson_feature['mapped_geometry'] = geojson.utils.map_tuples(lambda t: t, geojson_feature)

results in a geojson_feature that looks like:

{
  "geometry": {
    "coordinates": [-77.1291115237051, 38.7993076720178, 0],
    "type": "Point"
  },
  "id": "0",
  "mapped_geometry": {
    "geometry": {
      "coordinates": [-77.1291115237051, 38.7993076720178, 0],
      "type": "Point"
    },
    "properties": {
      "line": "blue",
      "marker-col": "#0000ff",
      "marker-sym": "rail-metro",
      "name": "Van Dorn Street"
    },
    "type": "Feature"
  },
  "properties": {
    "line": "blue",
    "marker-col": "#0000ff",
    "marker-sym": "rail-metro",
    "name": "Van Dorn Street"
  },
  "type": "Feature"
}

See this repl.

Are you saying that this behaviour is intended?

[bryik]

Looking at the docs again, I see map_tuple() says it "maps a function over all coordinates and returns a geometry of the same type" which implies Features should not be passed to it or if they are, that users should be prepared to receive only a geometry back.

However, the docstring of map_tuples() says it takes "A geometry or feature to extract the coordinates from" and Features have an explicit case in the code that does not raise an exception. Also when given a Feature, map_tuples() returns a Feature and not a geometry.

[rayrrr]

@bryik your additional notes helpful, and you are correct. Merging your PR in its current form. I was basing my earlier comments off of misleading docstrings, sorry. The docs/docstrings for utils.py functions need improvement...we have a related open issue: #110. While I would like these changes to be a bit more idiomatic, I will handle that in a quick follow-up PR and will give you a heads up on that. Thanks again.

[bryik]

@rayrrr Happy to help :-)

Thanks for the review and for maintaining this project!

[rayrrr]

@bryik #130 is the follow-up PR, have a look.