-
-
Notifications
You must be signed in to change notification settings - Fork 25.4k
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
TST improve metadata routing tests #29226
Conversation
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.
Awesome! Thanks for the PR to make the metadata testing easier.
Co-authored-by: Adam Li <[email protected]>
Co-authored-by: Adam Li <[email protected]>
Co-authored-by: Adam Li <[email protected]>
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.
LGTM. I only have one portion that I don't recall what we are doing there.
Co-authored-by: Guillaume Lemaitre <[email protected]>
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.
It LGTM and I just tried it out on #28494
@OmarManzoor you think you could have a look and hopefully merge maybe? |
@adrinjalali Sure I'll have a look soon. |
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.
LGTM. Thanks @adrinjalali
I just have one question.
This improves the tests for metadata routing in the way things are recorded and checked in test estimators.
This came up when working on Pipeline's
transform_input
(#28901) and made finding a few issues much easier.@glemaitre @OmarManzoor this should be easy-ish to merge, and would make the pipeline PR much smaller and easier to review, and should help @adam2392 with another SLEP6 PR.