Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Tree Keys #79

Closed
wants to merge 3 commits into from
Closed

Add Tree Keys #79

wants to merge 3 commits into from

Conversation

mitchelllisle
Copy link

Hi! Big fan of funcy - thought I'd add something that I find really useful and I believe fits in with the library. Basically, this function allows you to generate a list of keys within a nested dict.

This can then be used to iterate over a dictionary and make sure you're able to access any level of nested dicts. For example:

Given

payload = {
    "foo": "bar",
    "parent": {
        "val": True,
        "child": {
            "foo": "bar"
        }
    }
}

ltree_keys returns a list: [['parent'], ['parent', 'child'], ['parent', 'child', 'foo'], ['parent', 'val'], ['foo']]

That then works nicely with funcy.get_in and the like - for example: fn.lmap(lambda x: type(fn.get_in(payload, x)), keys)

Happy to make any updates or add anything else I may have missed.

@Suor
Copy link
Owner

Suor commented Oct 29, 2019

Hello, @mitchelllisle, thank for your involvement and sorry for slow feedback. There are, however, several issues with this:

  • your usage example might be much easier implemented using tree_nodes(), doesn't look like you need key paths here. I need to see a valid and common enough use case to consider adding this - I try to keep funcy scope sane.
  • fn.lmap(lambda x: type(fn.get_in(payload, x)), keys) is an inefficient because get_in() traverses those structures over and over. Lots of flattening inside also raise my consideration.
  • your implementation is not generic enough, see follow and children args in tree_nodes() and tree_leaves(). You are simply calling .get() and checking collection class.

@Suor Suor closed this Nov 16, 2019
@Suor
Copy link
Owner

Suor commented Nov 16, 2019

Feel free to reopen if you see something not covered above.

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.

None yet

2 participants