fix: ak.Record creation from iterable with nested dicts#3728
fix: ak.Record creation from iterable with nested dicts#3728pfackeldey merged 5 commits intoscikit-hep:mainfrom
ak.Record creation from iterable with nested dicts#3728Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
🚀 New features to boost your workflow:
|
|
The documentation preview is ready to be viewed at http://preview.awkward-array.org.s3-website.us-east-1.amazonaws.com/PR3728 |
ak.Record creation with nested dictsak.Record creation from iterable with nested dicts
pfackeldey
left a comment
There was a problem hiding this comment.
Thanks @ikrommyd, this is the correct fix. This additional check avoids calling the ak.Array constructor on a scalar (there we prohibit scalar promotion in to_layout). The else branch on the other hand wraps v into a list so that ak.to_layout properly operates on an iterable instead of a scalar. Also, this fix works for any nested level because the ak.Array constructor is called recursively.
This looks good to me, I only like to ask you to add the original issue as a test. Maybe it's also good to add a comment above the if condition saying "avoid dictionaries here, see issue #3723" as it may not seem obvious why this additional check is needed at first sight.
After that, this is ready to be merged 👍
|
Both done @pfackeldey! I did not fully understand why we loop manually over the dict and not use |
Closes #3723
I think
is_non_string_like_iterableis not enough here. It should also not be a dictionary cause dictionary is iterable.However, dicts I think should be wrapped into a single-element list like line 1864 does right below.