Skip to content

Conversation

@giadarol
Copy link
Member

Description

The survey plot was failing on lines containing Replicas. For replicas element._isthick needs to be deduced from the parent. Reusing xt.line._is_thick(element, line_or_env), which provides isthick looking at the parent when needed.

@giadarol giadarol requested a review from eltos October 11, 2025 21:36
Copy link
Member

@eltos eltos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, but a comment considering long-term maintainability:

Wouldn't it be more useful to fix this upstream? Since isthick is already a property, it could be handled there in a transparent way? Replica could store a reference to the parent element or line (as soon as added to the line).

Another option would be to make this private line._is_thick function public, and when accessing Replica.isthick to throw an exception hinting on using line.is_thick. That would prevent similar issues from slipping through unnoticed.

@giadarol
Copy link
Member Author

giadarol commented Oct 16, 2025

In general you can assume replica._parent to be there only if the replica is installed in a line and if the corresponding tracker has been built (the parents are resolved by line.build_tracker().

The function _is_thick(element, line) looks in the line to find the parent from the parent_name, also when element._parent is None.

Here we would have two options: using _is_thick(el, line) as proposed in this PR or ensuring that the line has a tracker (by building one if needed), which guarantees that all replicas have been resolved.

Both solutions would be fine with me.

@eltos eltos merged commit 197a8ab into main Oct 18, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants