Ensure separatrices, target plotted correctly if Dataset transposed

[johnomotani]

Create coordinates called 'xdim', 'ydim' and 'zdim' that depend only on x, y and z respectively, before applying geometry and re-naming the dimensions. This means that in plot_separatrices() and plot_targets() we can find renamed dimensions corresponding to x and y even if the Dataset has been transposed so that we cannot rely on the order of the dimensions.

This isn't exactly a beautiful solution, but I think it makes things more robust, without restricting what users can do with the BoutDataset.

[TomNicholas]

I'm not really a fan of adding extra coordinates to the dataset if we don't need to, if only because it's going to be confusing for users.

So this is intended to solve the problem of not knowing which dimensions correspond to the radial and poloidal directions on a plot if the 'x' and 'y' dimensions have been renamed? To do this we have to store some kind of record of which dimensions are radial and poloidal. We could have an attribute dedicated to this which is then consulted, which defaults to:

ds.attrs['plot_dims'] = {'radial': 'x', 'poloidal': 'y'}

The downside of this approach compared to yours is that it would have to be manually updated if someone renames their dims, whereas your coordinate-approach will update the coordinate to match automatically. I'm not sure which approach would be less confusing for users?

[ZedThree]

I'm not completely up to speed on what problem is trying to be solved here, but just be careful that "radial" and "poloidal" do imply a particular coordinate system, or set of coordinate systems, and don't apply to all BOUT++ uses. For example, linear plasma devices might use "azimuthal" instead of "poloidal".

[johnomotani]

@TomNicholas good points. I guess the question is what is the expected use-case? Probably most people will use 'toroidal' geometry, or a few other types (e.g. linear device, FCI) that could be added to xBOUT and maintained here. So having to manually update a dictionary translating x,y,z to the current dimension/coordinate names should not be a problem. I can't think of a real reason to rename the coordinates again.

The place this is actually needed is in the plotting routines that support plots in the R-Z plane in toroidal geometry, which are probably allowed to fail in other geometries (although I guess a cylindrical linear device might be able to use some of them). So cases where users are re-naming coordinates interactively and don't create a plot_dims attribute probably just don't expect those methods to work anyway. I'll update this.

[pep8speaks]

Hello @johnomotani! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-12-09 18:43:33 UTC

[johnomotani]

I had to add the new stuff to metadata so that it can be accessed through the DataArrays, and had to add a few variables since netCDF doesn't like dicts, but now pretty much implemented as @TomNicholas suggested.

[codecov-io]

Codecov Report

Merging #68 into master will increase coverage by 5.31%.
The diff coverage is 60%.

@@            Coverage Diff             @@
##           master      #68      +/-   ##
==========================================
+ Coverage   44.74%   50.05%   +5.31%     
==========================================
  Files           8        8              
  Lines         789      877      +88     
  Branches      152      178      +26     
==========================================
+ Hits          353      439      +86     
- Misses        380      381       +1     
- Partials       56       57       +1

Impacted Files	Coverage Δ
xbout/plotting/utils.py	4.91% <0%> (ø)	⬆️
xbout/geometries.py	74.44% <100%> (+1.82%)	⬆️
xbout/load.py	82.94% <0%> (+5.52%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f02e677...bc62e98. Read the comment docs.

[TomNicholas]

So cases where users are re-naming coordinates interactively and don't create a plot_dims attribute probably just don't expect those methods to work anyway.

Yes, exactly. If the user doesn't specify geometry='toroidal', they won't have 'R' and 'Z' coordinates, so the plotting method will fail with an appropriate error.

Thanks!