make feature geometry optional

[yellowcap]

What I am changing

According to the specs, the geometry attribute of a geojson Feature can be set to null. See https://datatracker.ietf.org/doc/html/rfc7946#section-3.2 . But the geometry in the Feature definition in geosjon-pydantic is required.

How I did it

Wrap the geometry value with an Optional[] tag in the feature definition.

How you can test it

I added a test for this case, but I am not sure if the test is sufficient the way I wrote it. I am not very familiar with pydantic.

Related Issues

Fixes #46

I came across this problem while using stac-fastapi, which uses stac-pydantic, which uses this library. The downstream issues are the following:
stac-utils/stac-pydantic#107
stac-utils/stac-fastapi#350

[vincentsarago]

thanks @yellowcap

@geospatial-jeff can you double check that this change is ok?

[geospatial-jeff]

LGTM!

[geospatial-jeff]

One note on this change. Because the Optional is implemented inside of the model any generic type that is passed in through Geom will be optional. If the Optional was implemented outside of the model in the typevar itself the generic type may be required.

Not really sure which is best. I guess there are use cases where you want to enforce that the geometry IS there, so maybe we should do the latter?

[vincentsarago]

Not really sure which is best. I guess there are use cases where you want to enforce that the geometry IS there, so maybe we should do the latter?

🤔 I'm not sure to follow, can we write tests or pseudo code?

[geospatial-jeff]

It's just a difference in where the Optional is declared.

# Optional defined inside pydantic model
class Feature(GenericModel, Generic[Geom, Props]):
    geometry: Optional[Geom]
    ....

# Or it can be defined in the `Geom` typevar
Geom = TypeVar("Geom", bound=Optional[Geometry])

If its declared inside the model, a user might set the Geom typevar to lets say geojson_pydantic.MultiPoint and that type will be coerced to Optional[MultiPoint]:

feat = Feature[MultiPoint, Props]
feat(geometry=None)

If it is declared in the typevar it's up to the user to define if the geometry is optional or not, which I think provides a better and more declarative interface:

feat = Feature[Optional[MultiPoint], Props]
feat(geometry=None)

Conversely, with the Optional defined in the typevar you can also specify that the geometry is required (this is not possible if Optional is baked into the pydantic model):

# Raises `ValidationError`
feat = Feature[MultiPoint, Props]
feat(geometry=None)

[vincentsarago]

I think I prefer the typeVar way!

any feedback @yellowcap ?

[yellowcap]

The question about where to put the Optional wrapper is a small but important point. Great explanation of the difference, it helped me a lot to understand what you were referring to. Thaks for that @geospatial-jeff!

I agree with @vincentsarago , the typeVar way is the way. It covers a more general use case of the geojson Feature type, and still makes the geom attribute optional in the default Feature type.

I changed the PR accordingly, please have another look and let me know if its ok like that.

[codecov-commenter]

Codecov Report

Merging #47 (8c408ab) into master (4608687) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #47   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          106       106           
=========================================
  Hits           106       106           

Flag	Coverage Δ
unittests	100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
geojson_pydantic/features.py	100.00% <100.00%> (ø)

---

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 4608687...8c408ab. Read the comment docs.

[vincentsarago]

thanks @yellowcap 🙏

I'll update the changelog and the documentation and try to cut a new release today 🥳

[yellowcap]

Good stuff! Thanks for the help @geospatial-jeff and @vincentsarago 😺