-
Notifications
You must be signed in to change notification settings - Fork 16
Fix testcases with coordinate issue #83
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
Fix testcases with coordinate issue #83
Conversation
maarten-ic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Prasad, thanks for looking into this!
Your changes look more complex than should be necessary, can you have a look at below comments?
imas/test/test_helpers.py
Outdated
| if any( | ||
| same_as.references for same_as in primitive.metadata.coordinates_same_as | ||
| ): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why we should reject error-bars from filling if they have same_as references, can you explain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am getting error with this specific case in distributions IDS. I will recheck this
try:
self._validate()
except ValidationError as exc:
# hide recursive stack trace from user
logger.debug("Original stack-trace of ValidationError: ", exc_info=1)
> raise exc.with_traceback(None) from None
E imas.exception.CoordinateError: Element `distribution[0]/profiles_2d[0]/grid/theta_straight_error_upper` has incorrect shape (5,): its coordinate in dimension 1 (`distribution[0]/profiles_2d[0]/grid/theta_straight`) has size 0.
imas/ids_toplevel.py:284: CoordinateError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed over Slack: I found the issue and pushed a commit to fix this. Please have a look and let me know if you have any comments 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Maarten
- Errorbars were not cleared by `unset_coordinate()`, this is fixed now - Use imas.util.tree_iter in unset_coordinate, which is more clear than visit_children and performs slightly better - Simplify some checks in `maybe_set_random_value()`
No description provided.