Closed danielballan closed 4 years ago
I should mention that this also lumps a different approach to #29, which was also necessary for JPLS. I think it will be necessary for all AD data---a very common use case, maybe the only use case in the wild.
Sorry for the late post @danielballan, your recollection of why we decided against using event descriptors to decide if tiff is suitable is correct. Although I think I also remember an argument (not sure if I made it or someone else did) that we wanted consistency across all suitcases and so I think they all inspect the arrays shape at present so perhaps that should also be considered.
Agreed, we should update this everything. I think the only other suitcase that filters by shape is CSV.
This looks good to go to me. Since this was originally my PR I can’t “approve” it. Ready for merge, @jklynch ?
In the original draft of this, if I recall correctly, @awalter-bnl and I both agreed that we should inspect that array's actual shape to decide whether it was suitable for writing as TIFF. In a better world, we would rely on the EventDescriptor for this, but at that time it was known that many EventDescriptor were not correct and we didn't want suitcase-tiff to be blocked by a database migration to fix them.
Now, databroker 1.0 gives us a hook to patch erroneous EventDescriptors using "transforms", and it also gives us new urgency to ensure they are correct. (If not, dask explodes.) It might be time to rely on them being correct here as well.
The immediately motivation for this issue is that suitcase-tiff blows up on strings, which it tries to cast to numpy arrays. Much safer to check the dtype and shape in the descriptor.