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:
|
|
I would be very disappointed if
|
|
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.
|
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 |
|
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. |
|
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 |
As I said I'm ready for delaying this PR so that |
This commit enables ruff rule PLC0415. Suggested by EarlMilktea: TeamGraphix#301 (comment)
|
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. |
I think, it is still preferable to random breakdowns. |
|
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.