search

scipp / scippnexus

h5py-like utility for NeXus files with seamless scipp integration
https://scipp.github.io/scippnexus/
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Handle issues with getting field dims more gracefully #99

Closed SimonHeybrock closed 1 year ago

SimonHeybrock commented 1 year ago

Getting field dims leads to reading info from the group. If this is incomplete this may raise, with similar or different problems than that of loading the entire group.

If loading the group fails, the latest scippnexus is falling back to loading as a DataGroup. Similarly, this change avoids problems when trying to load fields of a broken/incomplete group.

Also includes a minor change with fewer linebreaks in warnings to improve readability. We should probably consider logging warnings instead?

jl-wynen commented 1 year ago

I'm starting to wonder how to debug this. If you try to load a file and some parts don't get loaded as you would expect, it can be very difficult to figure out why. Would it help to log warning or info messages whenever the loader hits a fallback?

Also includes a minor change with fewer linebreaks in warnings to improve readability. We should probably consider logging warnings instead?

Maybe. But raising warnings has the advantage that users can react to them. They can, e.g., wrap their loading code in a warning filter and error out if the file is broken. This is a lot more difficult with logging.

SimonHeybrock commented 1 year ago

I'm starting to wonder how to debug this. If you try to load a file and some parts don't get loaded as you would expect, it can be very difficult to figure out why. Would it help to log warning or info messages whenever the loader hits a fallback?

We are warning when loading as a DataGroup, with the original exception test. In this case, the difference in the except branch is that dims would be default dims (('dim_0', ...), and I am not sure a warning would add much?

jl-wynen commented 1 year ago

and I am not sure a warning would add much?

Would it allow users to distinguish between a missing attr and a broken one?

SimonHeybrock commented 1 year ago

and I am not sure a warning would add much?

Would it allow users to distinguish between a missing attr and a broken one?

Probably in some cases. There is also a bunch of guessing based on shape going on, and things may fail if, e.g., a value dataset is missing, etc..

I will add a warning.