-
Notifications
You must be signed in to change notification settings - Fork 280
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 a check for reduced dimensionality slices #2842
Add a check for reduced dimensionality slices #2842
Conversation
# checks against it in the axes that are beyond its dimensionality. | ||
# This means that if we have a 2D dataset, *all* slices along z will | ||
# select all the zones. | ||
if self.axis >= dobj.ds.dimensionality: |
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 know folks that really love x-z
domains and do a lot of y
-slices :-)
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.
So I think in that case, we'll have set geometry = ("cartesian", "xzy")
and so when we slice along coord 2 it will still represent y
, right?
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'm pretty sure that's right, though a test would hurt, if it's easy enough to set up.
I don't think we have fake datasets in reduced dim, but you could use some amrvac dataset if there's no better option here, they come in many dimensionalities and geometries !
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.
left 2 questions. I'll be happy to approve once they are answered :)
# checks against it in the axes that are beyond its dimensionality. | ||
# This means that if we have a 2D dataset, *all* slices along z will | ||
# select all the zones. | ||
if self.axis >= dobj.ds.dimensionality: |
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'm pretty sure that's right, though a test would hurt, if it's easy enough to set up.
I don't think we have fake datasets in reduced dim, but you could use some amrvac dataset if there's no better option here, they come in many dimensionalities and geometries !
This adds a check to the
SliceSelector
. When we are slicing along an axis that is greater than or equal to the dimensionality of the dataset, we want to always say "yes" we are selecting a zone.Note that I did not modify the grid selection, since it should work out of the box with the way we clip to the edges.
There may be other objects that need this, but Slice is the most obvious one from my reading.
This should fix the Phantom Bugs in the other PRs, and I've also added a test (that fails without this changeset) to verify it's working.