Abstract out all axis information

#821 Merged at 3cb702b
  1. MattT

This abstracts out the axis name and identity information. This is the first step toward fixing right-handedness. Here is a script that demonstrates using this to fix righthandedness:


The before and after images are attached.


UPDATE 1: Fixed a few minor issues causing test failures. UPDATE 2: All tests pass.

Comments (14)

  1. MattT author

    @jzuhone should have added you as a reviewer, because this might help with getting better axis names for the FITS frontend.

  2. MattT author

    Note that, at present, this does not do the right thing for spherical and cylindrical coordinate system naming. In fact, there is a slight regression -- see attached, where I have sliced along phi in a spherical dataset.HexahedralMeshData_Slice_phi_B2.png

  3. chummels

    Looks like a step in the right direction. Glad to see this is being addressed!

    Do you think it makes sense to have helper functions for pulling the x_axis and y_axis from the user, just to make sure the input is sanitized before it messes things up? Like:

    ds.set_axes(x_axis, y_axis)

    or something like that? It may be a way of cleaning up x_axis and y_axis too, in that one only has to specify a 3-tuple instead of a 6-tuple. It seems more intuitive that way, but maybe I'm not grasping the whole problem and we're missing information if we go for this method.

    Good work, @MatthewTurk .

    1. MattT author

      Hi Cameron, I'm not sure I understand what you're suggesting? How would I use this function?

      1. chummels

        Will a user need to specify the full axis information each time as a 6-tuple as in your script?

        x_axis = { 'x' : 1, 'y' : 2, 'z' : 0, 0 : 1, 1 : 2, 2 : 0}

        I was simply suggesting that rather than passing it a dictionary to the coordinates structure, you have it build the coordinates from a function that only accepts 3 values so there is less room for a mixup (and it seems like the second set of coords are degenerate):

        ds.coordinates.set_xaxis(1,2,0) or ds.coordinates.set_xaxis(x=1,y=2,z=0)

        Or were you planning to have default axis information that is hidden away from the user and never needs to be manually set, and you're only exposing it here for the purposes of this PR?

        1. MattT author

          I exposed it here - this is resetting internal variables for the purpose of demonstration. It's not supposed to.be user-facing.

  4. MattT author

    I think as long as we're okay with the non-Cartesian stuff looking a bit funny for a while, this is ready to go in. @brittonsmith , any thoughts? Next PR will switch to right-handedness for PWs.

    1. Britton Smith

      Just so I'm clear, this is allowing us to fix righthandedness, but we are not actually fixing it in this PR?