Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only make additional Python checks if no schema errors #40

Closed
GraemeWatt opened this issue Nov 12, 2021 · 2 comments · Fixed by #41
Closed

Only make additional Python checks if no schema errors #40

GraemeWatt opened this issue Nov 12, 2021 · 2 comments · Fixed by #41
Assignees
Labels

Comments

@GraemeWatt
Copy link
Member

I've noticed a few recent error emails from the submission system with An unexpected error occurred: float() argument must be a string or a number, not 'NoneType' and an exception raised in Sentry (example) with Final attempt of process_saved_file for recid 1636571704 failed. Resetting to previous status. This happens because null values are given for some fields, e.g.

dependent_variables:
- header: {name: '$d^{2}\sigma/d\it{p}_{T}d\eta(mb \it{c}/GeV)$'}
  qualifiers:
  - {name: '$\it{p}_{T,jet}^{ch} (GeV/c)$', value: 'Jetptg R=0.2'}
  values:
  - value: 
    errors:
    - {symerror: , label: '$p_{T}$ statistical'}
    - {symerror: , label: '$p_{T}$ systematic'}
  - value: 
    errors:
    - {symerror: , label: '$p_{T}$ statistical'}
    - {symerror: , label: '$p_{T}$ systematic'}

Here, the value and symerror will show as None in Python, giving an uncaught exception TypeError: float() argument must be a string or a number, not 'NoneType' from error = float(error). Probably it would be easier just to make the additional Python checks such as check_for_zero_uncertainty if there are no validation errors from the JSON schema (i.e. revert a change made in dd5d463). Otherwise, the Python code making the additional checks cannot make any assumptions about the validity of the fields.

Could you please address this issue, @alisonrclarke, together with #39?

@GraemeWatt GraemeWatt added the bug label Nov 12, 2021
@alisonrclarke alisonrclarke self-assigned this Nov 15, 2021
@alisonrclarke
Copy link
Contributor

Instead of only doing the additional check is there are no schema errors, would it be OK to do them but wrap in try/except, ignoring any exceptions if there are already errors?

I just think it's helpful to the end user to show as many errors as possible first time.

@GraemeWatt
Copy link
Member Author

Yes, that's a better idea than missing out the additional checks completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants