Stack.py now preserves attrs

funkelab / gunpowder

A library to facilitate machine learning on multi-dimensional images.

https://funkelab.github.io/gunpowder/

MIT License

78 stars 56 forks source link

Stack.py now preserves attrs #210

Open griffbad opened 5 months ago

griffbad commented 5 months ago

took me a while to figure out why my attrs data (initially found from using extra_data in SpecifiedLocation) was not preserved at end of pipeline. This was it.

Pre-Merge Checklist:

[ ] if this PR adds a feature: request merge into the latest development branch (vX.Y-dev)
[X ] if this PR fixes a bug: request merge into the latest patch branch (patch-X.Y.Z)
[ ] PR branch name is short and describes the feature/bug addressed in this PR
[ ] commit messages are consistent
[ ] changes reviewed by another contributor
[ ] tests cover changes
[ ] all tests pass

pattonw commented 5 months ago

Thanks for bringing this to my attention!

The whole extra_data field needs to be reworked. We are moving away from using .attrs and instead just providing extra data in "non-spatial" arrays that are still managed by requests. This way there's no unexplained data being passed around.

In the mean time until .attrs is completely removed, I think this seems like a reasonable patch for the current behavior. I would only make sure that the value stored in .attrs remains a dictionary instead of being turned into a list as it is done here. Something like just assuming the set of keys in each batch do not overlap and then turning them into a larger dict with a union across the keys.