Update pybids compatibility flattening#79
Conversation
Removed the validate argument from layout initialization. Passing this previously had no effect as it wasn't used anywhere downstream nor did it throw any message about it. With this change, users can still pass the `validate` when initializing BIDSLayout without it doing anything since the method accepts **kwargs
pybids doesn't have a `.df` attribute to BIDSLayout, instead relying on a `to_df()` method. To avoid breaking this and making it more plug-and-play, add a method to mimic pybids behaviour and return the attribute value
Previous flattening, via the `.flatten()` method did not flatten the extra entities due to the type of the column. Expected StructType but got MapType instead. This commit: - Adds a method, `_flatten_extra_entities`, that performs the flattening, and adds it to the entity map that is used elsewhere, while moving the root and path columns to the end of the table. The latter is not strictly necessary, but moved it to keep the entity columns together. - Removes the TODO note, as "extra_entities" is no longer a column that needs to be considered
|
Hi — I prepared a follow-up patch for PR #79 locally, but I don’t have push access to the upstream branch Current PR head: Patch file: Summary of the patch:
If someone with push access can apply it, the commands should be: git checkout b2t/pybids-compat
git am pr79-local-head.patch
git push origin b2t/pybids-compat |
|
@2593803287-hue - No patch is attached and the commit you mention isn't visible anywhere reachable. Please open a PR from your fork if you'd like the changes reviewed. |
gkiar
left a comment
gkiar
left a comment
There was a problem hiding this comment.
@kaitj Thanks! Regarding this question:
This also facilitates the extraction of unique values within the extra entities column. Should we / do we want to support the full subject and extension convention in the compatibility layer?
This is currently supported, here, where we add both the short-name and long-name to the index.
LGTM!
4 participants
Main change in PR is to update the flattening of the
extra_entities. The.flatten()call ran into 2 issues:extra_entitiescolumn was aMapType, which pyarrow didn't end up flattening during my tests.This PR adds a new private method that performs the flattening by pulling out all the entities from
extra_entitiesand giving it its own column, before dropping theextra_entitiescolumn. This ends up being a closer match to pybids behaviour (I think) and allows for filtering by these extra entities when using the compatibility layer.This a rough summary of the column differences between the 3 tools (OG pybids, b2t before PR, b2t after PR):
Note
b2t uses abbreviations for
subjectandextension(pybids naming)fromandtowere in the still in the `extra_entities`` for b2t (before this pr).This also facilitates the extraction of unique values within the extra entities column. Should we / do we want to support the full
subjectandextensionconvention in the compatibility layer?Some additional minor things:
validateargument inBIDSLayout.__init__.to_df()method that mirrors theBIDSLayoutmethod in pybids. Probably not strictly necessary, but will ensure drop-in compatibility if anyone is using the method.