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

Fix compound regions #601

Merged
merged 3 commits into from Jan 15, 2020

Conversation

keflavich
Copy link
Contributor

Subcubes from region lists were untested and had some incorrect code

@coveralls
Copy link

coveralls commented Jan 15, 2020

Coverage Status

Coverage increased (+0.03%) to 87.938% when pulling 4f27d66 on keflavich:compound_regions_fix into 6e8d71b on radio-astro-tools:master.

@keflavich
Copy link
Contributor Author

+8.5% seems like a lot for one line....

@keflavich
Copy link
Contributor Author

Hit a related issue pretty fast, built on #595:

In [10]: scube = cube.subcube_from_regions(reg)
Traceback (most recent call last):
  File "<ipython-input-10-946720ff5a29>", line 1, in <module>
    scube = cube.subcube_from_regions(reg)
  File "/orange/adamginsburg/repos/spectral-cube/spectral_cube/spectral_cube.py", line 1991, in subcube_from_regions
    return masked_subcube.minimal_subcube(spatial_only=True)
  File "/orange/adamginsburg/repos/spectral-cube/spectral_cube/spectral_cube.py", line 1740, in minimal_subcube
    spatial_only=spatial_only)]
  File "/orange/adamginsburg/repos/spectral-cube/spectral_cube/spectral_cube.py", line 1778, in subcube_slices_from_mask
    wcs_tolerance=self._wcs_tolerance)
  File "/orange/adamginsburg/repos/spectral-cube/spectral_cube/masks.py", line 116, in include
    return self._include(data=data, wcs=wcs, view=view)
  File "/orange/adamginsburg/repos/spectral-cube/spectral_cube/masks.py", line 407, in _include
    return np.bitwise_and(result_mask_1, result_mask_2)
TypeError: operand type(s) all returned NotImplemented from __array_ufunc__(<ufunc 'bitwise_and'>, '__call__', dask.array<getitem, shape=(1920, 188, 62), dtype=float64, chunksize=(40, 28, 28)>, array([[[False, False, False, ..., False,
False, False],
...
        [False, False, False, ..., False, False, False]]])): 'Array', 'ndarray'

@keflavich
Copy link
Contributor Author

keflavich commented Jan 15, 2020

I guess dask hasn't implemented the bitwise_and operation?

ipdb> np.logical_and(result_mask_1, result_mask_1)
dask.array<logical_and, shape=(1920, 188, 62), dtype=bool, chunksize=(40, 28, 28)>                                                                                                                                                          
ipdb> np.bitwise_and(result_mask_1, result_mask_1)
*** TypeError: operand type(s) all returned NotImplemented from __array_ufunc__(<ufunc 'bitwise_and'>, '__call__', dask.array<getitem, shape=(1920, 188, 62), dtype=float64, chunksize=(40, 28, 28)>, dask.array<getitem, shape=(1920, 188, 62), dtype=float64, chunksize=(40, 28, 28)>): 'Array', 'Array'

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

Looks good to me (ignoring the dask issue) but maybe add a changelog entry if this is a bug that could have affected users?

@keflavich keflavich merged commit 8a4b213 into radio-astro-tools:master Jan 15, 2020
@keflavich keflavich deleted the compound_regions_fix branch January 27, 2020 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants