loicleyendecker / deeppath

MIT License
5 stars 4 forks source link

Improve flattening with * wildcard #8

Closed loicleyendecker closed 2 years ago

loicleyendecker commented 2 years ago

Removed all skips in tests as all functionalities should now be working.

I also rewrote dget to not use recursion as it was easier to make it work this way and I think it should be more efficient for very big structures (though probably not a use case).

1Mark commented 2 years ago

Will review soon 😁

1Mark commented 2 years ago
drusu55 commented 2 years ago

Heya, when is this going to be merged, and are you planning a new release after? Need deeppath's help and I'd rather have these changes as well hehe :D

loicleyendecker commented 2 years ago

Heya, when is this going to be merged, and are you planning a new release after? Need deeppath's help and I'd rather have these changes as well hehe :D

@drusu55 I'd like to have this merged in the next week or so at worse, but I have some other (cool, in my opinion) changes lined up, so it might be a big release.

loicleyendecker commented 2 years ago

Fyi, I tested with the added test, and I think the issue was just the lists were not properly handled. I get the point that there is still a conceptual problem when you expect maybe 3 entries but get only two (which maybe we could address with some stuff like {2, 3} to say you want at least 2 or up to 3 results, but also eww), but I think it is quite natural to just say that you want anything that matches with no regard for the actual number of matches.

1Mark commented 2 years ago

I think we should be ok to merge once

Thanks Loic :)

1Mark commented 2 years ago

only this is pending https://github.com/loicleyendecker/deeppath/pull/8#issuecomment-1059524180 do you want to do this later, if so i will merge now?

loicleyendecker commented 2 years ago

only this is pending [#8 (comment)](https://github.com/loicleyendecker/deeppath/pull

loicleyendecker commented 2 years ago

only this is pending #8 (comment) do you want to do this later, if so i will merge now?

Actually I think the changelog is updated on release automatically.