Allow specifying Bivariate levels #2474
Conversation
philippjfr
added
type: bug
|
Looks good to me. Happy to merge when the tests are green. |
philippjfr
force-pushed
the
bivariate_levels
branch
from
95ecc8e
to
21da6c8
Compare
|
Ready to merge, push build failed due to updated test data. |
| contour = Contours([[(-0.5, 0.416667, 0.5), (-0.25, 0.5, 0.5)], | ||
| [(0.25, 0.5, 0.5), (0.5, 0.45, 0.5)]], | ||
| contour = Contours([[(-0.5, 0.416667, 0.5), (-0.25, 0.5, 0.5), | ||
| (np.NaN, np.NaN, 0.5), (0.25, 0.5, 0.5), |
There was a problem hiding this comment.
I don't quite understand where the NaNs come from...
There was a problem hiding this comment.
contours now returns a Contours element with one array per level, if there's multiple contours at a level they will be NaN separated.
|
Change looks good to me - As long as you have checked that the notebooks with the updated test data now have the correct number of contours, I'm happy to merge once the tests are green. |
I have, also added unit tests to check this. |
|
Looks good and the pr tests are green. Merging. |
This PR allows specifying levels for the Bivariate element as requested in #2099. Additionally it also ensures that the contours operation actually generates the requested number of levels, which was not the case before. Probably changes some test data but it's a clear bug fix.