botify-labs / simpleflow

Python library for dataflow programming.
https://botify-labs.github.com/simpleflow/
MIT License
68 stars 24 forks source link

Circular dependencies hell #420

Closed jbbarth closed 11 months ago

jbbarth commented 1 year ago

I'm often hitting circular dependencies errors, and some colleagues have been as well. The nasty thing is that two persons can experiment different failures (things work for me, but not for my neighbour, although they have the same pyenv/python/libs/OS :grimacing: ; I don't know why).

Those issues are due to:

One trivial case to reproduce:

# happens on my machine (MacOS + Cpython 3.8.6)
# reproducible with "docker run -it python bash" + "pip install simpleflow"
python -c 'import swf.core'
# => ImportError: cannot import name 'ConnectedSWFObject' from partially initialized module 'swf.core' (most likely due to a circular import) (/usr/local/lib/python3.11/site-packages/swf/core.py)

I've just read this article and what they propose makes sense + they have an AST based test generator that allows to detect violations of the pattern they propose => I will take a shot at this to reorganise some imports, and move the remaining circular deps (if any) to local imports.

cc @ybastide let me know if you're against that

ybastide commented 1 year ago

Definitively ❤️, thanks!

jbbarth commented 1 year ago

First PR here: https://github.com/botify-labs/simpleflow/pull/421 -> moves "swf" into "simpleflow.swf.mapper" and solves the circular imports. I believe this PR is mandatory for fixing circular imports reliably.

Second PR here: https://github.com/botify-labs/simpleflow/pull/422 -> uses the set of tests in the article I mentioned above and rewrites the imports accordingly. Not sure it should be merged, curious what you think.

jbbarth commented 11 months ago

Fixed by #421 and #422