-
Notifications
You must be signed in to change notification settings - Fork 194
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
Add sample_coord for RegionNDMap #4088
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.
Thanks a lot @registerrier! Great to see that just moving the method up the hierarchy works. You could optionally simplify the test by removing the creation of the Table
.
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.
Thanks @registerrier! No other comments from my side...
@registerrier Merging #4085 introduced a merge conflict, would you mind rebasing this PR here? Thanks! |
cefb9aa
to
88d3cae
Compare
I just rebased. |
Signed-off-by: Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Terrier <rterrier@apc.in2p3.fr>
Signed-off-by: Terrier <rterrier@apc.in2p3.fr>
88d3cae
to
c9b2428
Compare
Signed-off-by: Terrier <rterrier@apc.in2p3.fr>
Codecov Report
@@ Coverage Diff @@
## master #4088 +/- ##
==========================================
- Coverage 94.02% 93.99% -0.03%
==========================================
Files 162 162
Lines 21459 21430 -29
==========================================
- Hits 20176 20144 -32
- Misses 1283 1286 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Thanks @registerrier, looks good to me!
Description
This pull request moves the
sample_coord
method to the baseMap
to allow sampling ofRegionNDMap
.For now the implementation fails with
HpxNDMap
, aNotImplmentedError
is therefore raised inHpxNDMap.sample_coord
.This should help #4087 and #4070 .
Dear reviewer