Skip to content

fix: warn if missing vector in transformation - #242

Merged
jokasimr merged 2 commits into
mainfrom
nxtransformation-warn
Sep 26, 2024
Merged

jokasimr merged 2 commits into
mainfrom
nxtransformation-warn

Conversation

@jokasimr

Copy link
Copy Markdown
Contributor

Fixes scipp/ess#306

  • Is NexusStructureError the right kind of error to raise if the transform is missing the vector attribute?

Comment thread src/scippnexus/base.py Outdated
try:
dg = maybe_transformation(self, value=dg, sel=sel)
except (sc.DimensionError, NexusStructureError) as e:
self._warn_fallback(e)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is actually the wrong warning, since it will say "failed to load as NXlog", but that part worked. The subsequent parsing of the log as a transform failed. Maybe moving the try/catch into maybe_transformation is better after all?

@jokasimr jokasimr Sep 26, 2024 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the test into maybe_transformation instead, and made it emit the warning if the transform loading errored out for any reason. Is that desirable?

Comment thread src/scippnexus/nxtransformations.py Outdated
Comment on lines +90 to +91
if self.attrs.get('vector') is None:
raise NexusStructureError('A transformation needs a vector attribute')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think using this error is ok.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using TransformationError?

Comment thread src/scippnexus/nxtransformations.py Outdated
Comment thread src/scippnexus/nxtransformations.py Outdated
Comment on lines +90 to +91
if self.attrs.get('vector') is None:
raise TransformationError('A transformation needs a vector attribute.')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better put this into self.vector?

@jokasimr
jokasimr force-pushed the nxtransformation-warn branch from 71dc6c8 to 0b7667d Compare September 26, 2024 11:37
@jokasimr
jokasimr merged commit cd17d10 into main Sep 26, 2024
@jokasimr
jokasimr deleted the nxtransformation-warn branch September 26, 2024 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix nightly builds

2 participants