yt-project / yt

Main yt repository
http://yt-project.org
Other
468 stars 280 forks source link

slice().to_frb() ignores aspect ratio #1023

Closed yt-fido closed 7 years ago

yt-fido commented 9 years ago

Originally reported by: maxhutch (Bitbucket: maxhutch, GitHub: maxhutch)


to_frb(width, resolution, height) ignores height when selecting which data to use for the window. It does, however, produce an FRB with the given resolution, which can have non-unity aspect ratio.

In the attached ipynb, the three visualizations should be the same (up to an overall rotation). The bug prevents the third visualization from displaying a circle.


yt-fido commented 8 years ago

Original comment by chummels (Bitbucket: chummels, GitHub: chummels):


Merged in ngoldbaum/yt (pull request #2074)

Fix copy/paste error in to_frb method. Closes #1023.

yt-fido commented 8 years ago

Original comment by chummels (Bitbucket: chummels, GitHub: chummels):


Fix copy/paste error in to_frb method. Closes #1023.

yt-fido commented 8 years ago

Original comment by chummels (Bitbucket: chummels, GitHub: chummels):


Merged in ngoldbaum/yt (pull request #2074)

Fix copy/paste error in to_frb method. Closes #1023.

yt-fido commented 8 years ago

Original comment by chummels (Bitbucket: chummels, GitHub: chummels):


Fix copy/paste error in to_frb method. Closes #1023.

yt-fido commented 8 years ago

Original comment by chummels (Bitbucket: chummels, GitHub: chummels):


Merged in ngoldbaum/yt (pull request #2074)

Fix copy/paste error in to_frb method. Closes #1023.

yt-fido commented 8 years ago

Original comment by chummels (Bitbucket: chummels, GitHub: chummels):


Merged in ngoldbaum/yt (pull request #2074)

Fix copy/paste error in to_frb method. Closes #1023.

yt-fido commented 8 years ago

Original comment by Nathan Goldbaum (Bitbucket: ngoldbaum, GitHub: ngoldbaum):


Oh, I'm sorry, I completely forgot about that. In general it's best to just fix issues yourself if you have a solution.

yt-fido commented 8 years ago

Original comment by Jonathan Slavin (Bitbucket: jdslavin, GitHub: jdslavin):


Hi Nathan,

Sure. I'd been considering proposing a pull request, as you may recall from my post on the yt-users list. At that time you offered to submit it for me and I responded that I wanted to take you up on that. I do have a little experience with pull requests and I'll be happy to submit them in the future if a similar situation comes up.

Jon

yt-fido commented 8 years ago

Original comment by Nathan Goldbaum (Bitbucket: ngoldbaum, GitHub: ngoldbaum):


@jdslavin Thanks for the suggestion! In the future can you propose code fixed like this as a pull request? That way we will be more likely to see it. I've gone ahead and offered your fix as a pull request here: https://bitbucket.org/yt_analysis/yt/pull-requests/2074

yt-fido commented 8 years ago

Original comment by Jonathan Slavin (Bitbucket: jdslavin, GitHub: jdslavin):


I think I found the source of the problem. In data_objects/data_containers.py: 957 elif iterable(height): 958 h, u = height 959 height = self.ds.quan(w, input_units = u)

If line 959 is replaced with: height = self.ds.quan(h, input_units = u)

then it works