Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: ensure YTSlice.coord has units #5000

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

chrishavlin
Copy link
Contributor

@chrishavlin chrishavlin commented Sep 23, 2024

PR Summary

Found that for sph data, calling ds.slice(axis, center).to_frb(...)[field] would error if the center argument was a plain float. For other dataset types (and other data objects), a plain float is assumed to be in units of code_length. Easiest fix is to ensure that YTSlice.coord attribute is a unyt object, following the other data objects.

Here's the stack trace from the error on main:

import yt
ds = yt.load_sample("snapshot_033")
frb = ds.slice(0, 0.25).to_frb(ds.domain_width[0], (64, 64))
print(frb["gas", "density"])
# Traceback (most recent call last):
#   File "/Users/chavlin/.pyenv/versions/yt_dev/lib/python3.10/site-packages/IPython/core/interactiveshell.py", line 3548, in run_code
#     exec(code_obj, self.user_global_ns, self.user_ns)
#   File "<ipython-input-3-76bca84cf85c>", line 6, in <module>
#     print(frb["gas", "density"])
#   File "/Users/chavlin/src/yt_/yt_dev/yt/yt/visualization/fixed_resolution.py", line 209, in __getitem__
#     return self.get_image(item)
#   File "/Users/chavlin/src/yt_/yt_dev/yt/yt/visualization/fixed_resolution.py", line 213, in get_image
#     self._generate_image_and_mask(key)
#   File "/Users/chavlin/src/yt_/yt_dev/yt/yt/visualization/fixed_resolution.py", line 179, in _generate_image_and_mask
#     buff, mask = self.ds.coordinates.pixelize(
#   File "/Users/chavlin/src/yt_/yt_dev/yt/yt/geometry/coordinates/cartesian_coordinates.py", line 238, in pixelize
#     buff, mask = self._ortho_pixelize(
#   File "/Users/chavlin/src/yt_/yt_dev/yt/yt/geometry/coordinates/cartesian_coordinates.py", line 554, in _ortho_pixelize
#     data_source.coord.to("code_length").v,
# AttributeError: 'float' object has no attribute 'to'

PR Checklist

  • [ ] New features are documented, with docstrings and narrative docs N/A
  • Adds a test for any bugs fixed. Adds tests for new features.

@chrishavlin chrishavlin added this to the 4.4.0 milestone Sep 23, 2024
@neutrinoceros
Copy link
Member

random github markdown tip of the day:
python-traceback is a supported syntax coloring scheme:

Traceback (most recent call last):
  File "/Users/chavlin/.pyenv/versions/yt_dev/lib/python3.10/site-packages/IPython/core/interactiveshell.py", line 3548, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-3-76bca84cf85c>", line 6, in <module>
    print(frb["gas", "density"])
  File "/Users/chavlin/src/yt_/yt_dev/yt/yt/visualization/fixed_resolution.py", line 209, in __getitem__
    return self.get_image(item)
  File "/Users/chavlin/src/yt_/yt_dev/yt/yt/visualization/fixed_resolution.py", line 213, in get_image
    self._generate_image_and_mask(key)
  File "/Users/chavlin/src/yt_/yt_dev/yt/yt/visualization/fixed_resolution.py", line 179, in _generate_image_and_mask
    buff, mask = self.ds.coordinates.pixelize(
  File "/Users/chavlin/src/yt_/yt_dev/yt/yt/geometry/coordinates/cartesian_coordinates.py", line 238, in pixelize
    buff, mask = self._ortho_pixelize(
  File "/Users/chavlin/src/yt_/yt_dev/yt/yt/geometry/coordinates/cartesian_coordinates.py", line 554, in _ortho_pixelize
    data_source.coord.to("code_length").v,
AttributeError: 'float' object has no attribute 'to'

Copy link
Member

@neutrinoceros neutrinoceros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me unfortunately it seems I cannot compile yt these days (I assume because I'm being too aggressive with upgrading my system dependencies), so I'm not able to verify that your test captures the problem. Nonetheless it seems trivial enough that I can approve to give you the option of self-merging here.

@@ -1106,7 +1106,7 @@ def validate_3d_array(obj):
def validate_float(obj):
"""Validates if the passed argument is a float value.
Raises an exception if `obj` is a single float value
Raises an exception if `obj` is not a single float value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch

@chrishavlin
Copy link
Contributor Author

Nonetheless it seems trivial enough that I can approve to give you the option of self-merging here.

It's not a critical fix, so I'll let it hang out for now and self merge at the end of the day if no one else jumps in.

@jzuhone jzuhone merged commit b4cc03c into yt-project:main Sep 25, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants