Conversation
WalkthroughAdds per-container truncation control via a new Changes
Sequence Diagram(s)(omitted — representation/truncation changes do not alter control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
flixopt/results.py (1)
249-251: Truncated repr for results containers — good default; consider making max items configurable.Enabling truncation on components/buses/effects/flows improves readability for large results. As a small enhancement, consider a per-container or global knob (e.g., via ContainerMixin or CONFIG) to adjust the truncate threshold without code changes. Also, a tiny unit test asserting the presence of the “... (+N more)” suffix would be useful.
Also applies to: 254-255, 257-257, 272-272
flixopt/effects.py (1)
461-463: Forwarding truncate_repr in EffectCollection — LGTM; add a brief docstring.The constructor change is sound. Add a short docstring to document truncate_repr for discoverability.
Apply this minimal docstring:
def __init__(self, *effects: Effect, truncate_repr: bool = False): - super().__init__(element_type_name='effects', truncate_repr=truncate_repr) + """ + Initialize the collection. + Args: + *effects: Effects to register. + truncate_repr: If True, limit repr listing length in grouped views. + """ + super().__init__(element_type_name='effects', truncate_repr=truncate_repr)flixopt/flow_system.py (1)
178-183: Consistent truncated repr for public containers — 👍; optional configurability.Setting truncate_repr=True for components/buses/effects/flows aligns system-wide. Consider making the truncate threshold configurable (e.g., via a ContainerMixin arg or CONFIG) so users can widen/narrow listings without code edits.
Also applies to: 804-804
flixopt/structure.py (3)
924-935: New truncate_repr flag in ContainerMixin — solid plumbing; add optional per-container limit.Flag storage is straightforward. Consider an optional repr_max_items to control truncation per container without touching call sites later.
Apply this small, backward-compatible extension:
class ContainerMixin(dict[str, T]): @@ - def __init__( + def __init__( self, elements: list[T] | dict[str, T] | None = None, element_type_name: str = 'elements', - truncate_repr: bool = False, + truncate_repr: bool = False, + repr_max_items: int = 10, ): @@ self._element_type_name = element_type_name - self._truncate_repr = truncate_repr + self._truncate_repr = truncate_repr + self._repr_max_items = repr_max_items
1005-1034: _get_repr implementation — clear and efficient; reuse a per-instance default.Logic and natural sort look good. If you add _repr_max_items, let callers omit max_items yet still pick up the per-instance default.
Minimal tweak:
- def _get_repr(self, truncate: bool = False, max_items: int = 10) -> str: + def _get_repr(self, truncate: bool = False, max_items: int | None = None) -> str: @@ - if truncate and len(sorted_names) > max_items: + limit = max_items if max_items is not None else getattr(self, "_repr_max_items", 10) + if truncate and len(sorted_names) > limit: # Show truncated list - for name in sorted_names[:max_items]: + for name in sorted_names[:limit]: r += f' * {name}\n' - r += f' ... (+{len(sorted_names) - max_items} more)\n' + r += f' ... (+{len(sorted_names) - limit} more)\n'
1243-1247: Grouped container formatting honors truncation flag — nice.Works as intended. Minor nit: isinstance(container, ContainerMixin) would read clearer than hasattr, but current duck-typing is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
flixopt/effects.py(1 hunks)flixopt/flow_system.py(2 hunks)flixopt/results.py(2 hunks)flixopt/structure.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
flixopt/flow_system.py (2)
flixopt/structure.py (1)
ElementContainer(1043-1052)flixopt/effects.py (1)
EffectCollection(454-631)
flixopt/results.py (2)
flixopt/structure.py (2)
ResultsContainer(1055-1064)solution(159-182)flixopt/flow_system.py (1)
flows(799-805)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
🔇 Additional comments (1)
flixopt/structure.py (1)
1038-1041: Never-truncated direct repr — good choice.Keeps repr(element_container) complete while grouped views can truncate. No action needed.
1. ContainerMixin in structure.py - Changed truncate_repr from bool to int | None - None = show all items (no truncation) - Integer value = maximum number of items to show in repr - Default is None (no truncation) 2. Updated _get_repr() method - Simplified parameters from (truncate: bool, max_items: int) to just (max_items: int | None) - Uses instance's _truncate_repr as default if max_items is not provided - Cleaner logic: truncates only when limit is not None and limit > 0 3. Updated __repr__() method - Now simply calls self._get_repr() without arguments - Respects the instance's truncate_repr setting 4. Simplified _format_grouped_containers() - Removed the conditional logic checking for _truncate_repr - Now just calls repr(container) which automatically respects each container's setting 5. Updated all call sites - flow_system.py: Changed truncate_repr=True → truncate_repr=10 (4 locations) - results.py: Changed truncate_repr=True → truncate_repr=10 (4 locations) - effects.py: Changed truncate_repr: bool = False → truncate_repr: int | None = None and added docstring
1 participant
Description
Add truncation to the repr of containers
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit