Skip to content

Conversation

@rix1
Copy link

@rix1 rix1 commented May 8, 2024

This let us do inversion of control, passing in a callback function that gives more flexibility into how the node is rendered when using show() and save2file(). Essentially replaces the need for data_property - but we should probably keep it for backwards compatibility.

The callback function right now expect a string to be returned. I was thinking on maybe supporting None which could also replace the need for the filter argument: Return None to avoid the Node to be rendered. Let me know what you think @liamlundy or @caesar0301 :)

Example:

def render_node(node):
  if node.is_leaf():
    return node.my_property
  return node.some_other_property

tree.show(render_node=render_node)

See unit test in 626f439 for an actual example.

Checklist

  • I have formatted my code with Black I pass the local validation with scripts/flake8.sh.
  • I have updated docs: Please point me in the right direction here - I've added a docstring, but I'm not sure what/how/where to document the updated show() API.

This let us do inversion of control[1], passing in a callback function
that gives more flexibility into how the node is rendered when using
show() and save2file().

**Example:**

```py
def render_node(node):
  if node.level > 2:
    return node.my_property
  return node.some_other_property

tree.show(render_node=render_node)
```

[1]: https://en.wikipedia.org/wiki/Inversion_of_control
@rix1
Copy link
Author

rix1 commented May 8, 2024

Just added some unit tests.

@rix1 rix1 force-pushed the rix1/add-render-node branch from 626f439 to 5a3df96 Compare May 8, 2024 22:39
@BaxHugh
Copy link

BaxHugh commented Feb 27, 2025

I'd find this useful. I see this was created almost a year ago, any updates on getting this merged and released?
@caesar0301, @liamlundy @rix1

@caesar0301
Copy link
Owner

I'd find this useful. I see this was created almost a year ago, any updates on getting this merged and released? @caesar0301, @liamlundy @rix1

Same to me, I will spend time to fix the conflicts first, and add more uts.

@rix1
Copy link
Author

rix1 commented Jun 2, 2025

I'd find this useful. I see this was created almost a year ago, any updates on getting this merged and released? @caesar0301, @liamlundy @rix1

Same to me, I will spend time to fix the conflicts first, and add more uts.

Let me know if there's anything I can do to help :)

Happy to rebase and fix conflicts

@frukto
Copy link

frukto commented Aug 27, 2025

This would indeed be very helpful, @rix1 can you rebase again. Maybe it gets merged this time 🤷

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.

4 participants