Uh oh!
There was an error while loading. Please reload this page.
Resolve import loops - #301
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## master #301 +/- ##
==========================================
- Coverage 79.83% 79.82% -0.01%
==========================================
Files 36 36 Lines 5429 5428 -1 ==========================================
- Hits 4334 4333 -1
Misses 1095 1095 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
thierry-martinez
commented
Jun 13, 2025
I would be very disappointed if
|
EarlMilktea
commented
Jun 13, 2025
I really understand your opinion but still import loop is what we should not have.
This could be possible if
This is 100 % impossible since it immediately results in import loop. Let's find our point of compromise. |
This commit proposes a lighter fix than TeamGraphix#301 to resolve the import loop between `pretty_print` and `command`.
This commit introduces a lighter fix than TeamGraphix#301 to resolve the import loop between `pretty_print` and `command`. The import loop originated from: - In `command.py`: `from graphix.pretty_print import DataclassPrettyPrintMixin` - In `pretty_print.py`: `from graphix import command` If `pretty_print.py` was loaded first, then the statement `from graphix import command` would trigger the loading of `command.py` before `pretty_print` was fully initialized. As a result, the import `from graphix.pretty_print import DataclassPrettyPrintMixin` would fail because `DataclassPrettyPrintMixin` had not yet been defined. This commit resolves the issue by replacing the import in `command.py` with `from graphix import pretty_print`. This allows the two modules to depend on each other without causing a cyclic import failure.
thierry-martinez
commented
Jun 20, 2025
I agree with you that we should consider import loops to be bugs, and we want to resolve them so that modules can be imported in any order without causing random failures. I propose in #304 a lighter fix to resolve the import loop between |
EarlMilktea
commented
Jun 20, 2025
Checked #304 . While it would be sufficient for now, I still strongly believe that, at least in the future, we need to consider loop-free (DAG) import structure and actually this is possible by transforming I hope we someday reach agreement. |
thierry-martinez
commented
Jun 20, 2025
Python doesn't forbid cycles in the dependency graph, only import-time execution cycles are problematic. Moreover, the key issue isn’t whether something is implemented inside a class or a function: object-oriented design can be helpful in many cases. The deeper problem lies in having a large, central module like Conversely, naive object-oriented design can also lead to fragmentation: spreading the implementation of a single functionality across multiple classes or modules. Grouping related algorithmic parts can actually improve cohesion and clarity. For example, having a dedicated |
EarlMilktea
commented
Jun 21, 2025
As I said I'm ready for delaying this PR so that |
This commit enables ruff rule PLC0415. Suggested by EarlMilktea: TeamGraphix#301 (comment)
thierry-martinez
commented
Jun 23, 2025
The rule PLC0415 could be desirable (and we can easily comply with it; see #307), but it does not require a DAG structure. As I said, only import-time execution cycles are problematic, and I think that requiring a DAG structure will reduce the code’s modularity and maintainability by forcing things to be grouped or separated for purely technical reasons, regardless of the logic of the code. |
EarlMilktea
commented
Jun 24, 2025
I think, it is still preferable to random breakdowns. |
thierry-martinez
commented
Jun 24, 2025
We agree that import-time execution cycles should be considered as bugs, and we want to resolve them so that modules can be imported in any order without causing random failures. PR #304 resolves a bug preventing |
* Enable ruff rule PLC0415 This commit enables ruff rule PLC0415. Suggested by EarlMilktea: #301 (comment) Introduce `repr_mixins` for `DataclassReprMixin` and `EnumReprMixin`.
Context (if applicable):
This PR fixes another import loop causing problems in #280 .
Description of the change:
prety_print.pyruffpyproject.toml.After merging this PR, for all the typed files,
importwill become completely loop-free.