drivendataorg / erdantic

Entity relationship diagrams for Python data model classes like Pydantic
https://erdantic.drivendata.org/
MIT License
283 stars 20 forks source link

Modality `zero` should only be determined by Optional typing #90

Closed ion-elgreco closed 3 months ago

ion-elgreco commented 10 months ago

I believe that the current way of defining the modality is not fully correct. If the cardinality is many, then the modality will become zero. But this means you will never get one-to-many. I propose that the modality gets determined to only check if a field is nullable and not also if the cardinality is many.

https://github.com/drivendataorg/erdantic/blob/b618ab54593d3b89853c2ce22f0b47f8bec41255/erdantic/erd.py#L56C1-L58C10

jayqi commented 10 months ago

I'm coming around to the modality not being affected by whether the field type is a collection. However, I'm wondering if perhaps the "one" symbol should not be shown?

This means one-or-many will still be unspecifiable, but there is not straightforwardly a way to do so with normal Python types. Collections can be empty, but perhaps it's a level of specificity that we don't want. This StackOverflow post's responses suggest that using the "many" notation without specifying "one" or "zero" seems to be better.

jayqi commented 3 months ago

This has been updated in v1.0.0.

A type annotation like List[Target] will have cardinality of many but a modality of unspecified. The arrow shape will only be crow. You can see this reflected in the example diagram at the top of the README: the edge from Party.members to Adventurer has only a crow arrowhead.

v1.0.0 is temporarily only available as a pre-release version (pip install erdantic --pre).