Skip to content

Conversation

@civilx64
Copy link
Contributor

closes #107

@civilx64 civilx64 requested a review from aothms November 26, 2025 20:29
Copy link
Collaborator

@aothms aothms left a comment

Choose a reason for hiding this comment

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

Really nice, but have a look at the two scenario suggestions. I think the schema still allows for some situations we need to check.

if 'npath' in inspect.getargs(fn.__code__).args:
kwargs = kwargs | {'npath': current_path}
top_level_index = current_path[0] if current_path else None
max_index = len(current_path) - 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is exactly related to my comment on #468

We should not be doing this, because in this way we're linking the wrong instance to the outcome presentation. We just really should make sure that the stack of values we build forms a proper tree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something needs to be done because this feature file generates an IndexError as-is. The code tries to access element at index 1 when there is a single value in the list. I'll review your comment for clues. Maybe the Given statement needs to be revised.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, the IndexError makes sense in #468 based on the Given statement. I'm working through that now & will use lessons learned to resolve this PR.

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