[yt-dev] Sequence of buff_size

[Yi-Hao Chen]

On Fri, Apr 1, 2016 at 11:23 AM, Matthew Turk <matthewturk at gmail.com> wrote:

> On Fri, Apr 1, 2016 at 11:21 AM, Nathan Goldbaum <nathan12343 at gmail.com>
> wrote:
> >
> >
> > On Fri, Apr 1, 2016 at 11:11 AM, Matthew Turk <matthewturk at gmail.com>
> wrote:
> >>
> >> Hi Yi-Hao,
> >>
> >> The actual usage of _MPL.c shouldn't be user-facing.  So I think as
> >> long as the things that are public APIs (such as the data returned
> >> from a FixedResolutionBuffer object) remain the same, I am a strong +1
> >> on this.
> >
> >
> > The trouble is, if we want to make the user interface match what people
> > expect, i.e. buff_size is (nx, ny), where nx it the number of pixels
> along x
> > and ny is the number of pixels along y, we'll need to change the behavior
> > here.
>
> Hmmm .. but we transpose in most places, like
> yt/geometry/coordinates/cartesian_coordinate_handler.py .
>

I think the transpose of the image array is to fit the behavior of
imshow(), which needs an array with the shape (ny, nx) to display nx pixels
along x axis and ny pixels along y axis.

I assumed generally people expect buff_size is (nx, ny). But if they really
use non-square buff_size, they probably should have figured it out by trial
and error and used (ny, nx) instead. The proposed fix will change this
behavior.


> >
> >>
> >>
> >> -Matt
> >>
> >> On Fri, Apr 1, 2016 at 10:19 AM, Yi-Hao Chen <ychen at astro.wisc.edu>
> wrote:
> >> > Hi all,
> >> >
> >> > Running this test script
> >> > http://paste.yt-project.org/show/6379/
> >> > will result in this image
> >> > https://slack-files.com/T042F73QM-F0X0YJE3V-4a579d9431
> >> >
> >> > Apparently, the sequence of buff_size is switched so that
> >> >
> >> > set_buff_size((8,4))
> >> >
> >> > gives a 4 by 8 image.
> >> >
> >> > I have traced down the codes and found the following lines are the
> cause
> >> > of
> >> > the problem.
> >> >
> >> >
> https://bitbucket.org/yt_analysis/yt/src/d18f33211199f71e2fac4d927307b015d513a328/yt/utilities/lib/pixelization_routines.pyx?at=yt&fileviewer=file-view-default#pixelization_routines.pyx-61
> >> >
> >> >
> >> >
> https://bitbucket.org/yt_analysis/yt/src/d18f33211199f71e2fac4d927307b015d513a328/yt/utilities/lib/pixelization_routines.pyx?at=yt&fileviewer=file-view-default#pixelization_routines.pyx-216
> >> >
> >> > If rows=nx and cols=ny, the two functions (pixelize_cartesian and
> >> > pixelize_off_axis_cartesian) should take rows first and then cols.
> >> >
> >> > The same API has been around in _MPL.c since 2010 and thus changing it
> >> > will
> >> > be backward incompatible.
> >> >
> >> > However, to be consistent with other parts of the code (e.g. here,
> here,
> >> > here, and here), in which buff_size = (nx, ny) is assumed and used, I
> >> > propose to change the behavior of the above two functions.
> >
> >
> > I'm +0.5 on this, but am nervous that it will change the results of user
> > scripts. I'd like the API to match naive expectations though...
> >
> >>
> >> >
> >> > What do you think? Anyone has other suggestions?
> >
> >
> > The other alternative is to document the current (annoyingly backwards)
> > behavior.
>
If we want to keep the current behavior and document it, many parts of the
code will need to be changed as well.


> >
> >>
> >> >
> >> > Thanks!
> >> > Yi-Hao
> >
> >
> > Thank you for your detective work on this, Yi-Hao!
> >
> >>
> >> >
> >> > _______________________________________________
> >> > yt-dev mailing list
> >> > yt-dev at lists.spacepope.org
> >> > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >> >
> >> _______________________________________________
> >> yt-dev mailing list
> >> yt-dev at lists.spacepope.org
> >> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >
> >
> >
> > _______________________________________________
> > yt-dev mailing list
> > yt-dev at lists.spacepope.org
> > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >
> _______________________________________________
> yt-dev mailing list
> yt-dev at lists.spacepope.org
> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.spacepope.org/pipermail/yt-dev-spacepope.org/attachments/20160401/cfe3aab6/attachment.html>