Type hints in graph protocol could cause issues with type checker

[petergtz]

trafficstars

Hi @adam2392,

just as a heads-up: By using type-hints in

https://github.com/py-why/dodiscover/blob/3c4f8d251dcf080c4bce83292affff6cd46f335d/dodiscover/_protocol.py#L10

it could be that type-checkers will give a type violation when passing in a networkx Graph for this protocol, because networkx does not use type-hints. We've tried that originally for

https://github.com/py-why/dowhy/blob/ead8d47102f0ac6db51d84432874c331fb84f3cb/dowhy/gcm/graph.py#L26-L30

and then removed them again, because IntelliJ would show an error, and I'd assume other type-checkers will do the same.

Feel free to close this issue after reading, just wanted to make you aware of it.

[adam2392]

My current codebase developing dodiscover and its unit tests do not show an error rn because I use pywhy-graphs, but yeah you're right... an issue for networkx integration. However, I would say the typing additions will really help bring the pywhy ecosystem together and help explicitly formulate what the interfaces need to be. I think though this is def an issue rn due to networkx inertia, and we would have to document the issues here and limitations of using mypy and other type-checkers that will show an error... (?)

I'll keep the issue open to remind us of this issue.

Re types in networkx: https://github.com/networkx/networkx/pull/4014 seems to have active discussion. Perhaps pywhy should formulate an opinion and we can weigh in?

[petergtz]

Re types in networkx: https://github.com/networkx/networkx/pull/4014 seems to have active discussion. Perhaps pywhy should formulate an opinion and we can weigh in?

Good idea. Just did: https://github.com/networkx/networkx/pull/4014#issuecomment-1232779825