BUG: fix reading coordinates in Fargo3DReader.read
#384
Conversation
| grid_shape = n3, n1, n2 | ||
| shift = n2 // 2 | ||
| if geometry_str == "cylindrical": | ||
| grid_shape = n3, n1, n2 | ||
| tuple_transposition = (1, 2, 0) | ||
| shift = n2 // 2 | ||
| elif geometry_str == "spherical": | ||
| grid_shape = n2, n1, n3 | ||
| tuple_transposition = (1, 0, 2) | ||
| shift = n3 // 2 |
There was a problem hiding this comment.
There should be an else: raise NotImplementedError (even if it's currently unreacheable) to guard against NameErrors
nonos/api/analysis.py
Outdated
| else: | ||
| return im |
There was a problem hiding this comment.
why is this return conditional, and why would it depend on the seemingly unrelated title argument ?
Also note that returning early in this style makes the filename argument a no-op, which is probably not desired.
There was a problem hiding this comment.
this condition is exactly the same as the one just before for 2D images.
2D case :
im = plot
if title is not None:
colorbar by default
else:
return im # to have more flexibility on what you want in the colorbar, using im
I just reused this already existing structure for the 1D case, because I needed to have more flexibility on it.
There was a problem hiding this comment.
then my comment also applies there. I think you are effectively copy-pasting a bug here
neutrinoceros
force-pushed
the
return-plot-1d
branch
from
52e3c79 to
681dc32
Compare
neutrinoceros
changed the title
Fargo3DReader.read
titleargument in plot() function: return artist to be consistent with the 2D case