radio-astro-tools / spectral-cube

Library for reading and analyzing astrophysical spectral data cubes
http://spectral-cube.rtfd.org
BSD 3-Clause "New" or "Revised" License
95 stars 62 forks source link

Avoid non-lazy reading #783

Closed keflavich closed 2 years ago

keflavich commented 2 years ago

This is another independent attempt to solve #774/775

keflavich commented 2 years ago

floordiv was incorrectly being supported in some cases; it shouldn't work!

In [4]: np.array([5])//(2*u.K)
Traceback (most recent call last):
  File "<ipython-input-4-2cff403b3048>", line 1, in <module>
    np.array([5])//(2*u.K)
  File "/orange/adamginsburg/miniconda3/envs/python39/lib/python3.9/site-packages/astropy/units/quantity.py", line 466, in __array_ufunc__
    converters, unit = converters_and_unit(function, method, *inputs)
  File "/orange/adamginsburg/miniconda3/envs/python39/lib/python3.9/site-packages/astropy/units/quantity_helper/converters.py", line 192, in converters_and_unit
    raise UnitConversionError(
UnitConversionError: Can only apply 'floor_divide' function to dimensionless quantities when other argument is not a quantity (unless the latter is all zero/infinity/nan)
keflavich commented 2 years ago

ah shoot no it should be supported. I have to fix it.

keflavich commented 2 years ago

I think this solves a big part of #774/#775 now. None of the tests explicitly test for data loading - I don't think I know how to do that right now - but they circumvent some of the more problematic cases of u.Quantity(self)

keflavich commented 2 years ago

I had to totally remove the "expected failure" test for floordiv because it was leaving files open - I don't understand that at all.

Otherwise, this goes a long way to avoiding the early reads of full cubes

e-koch commented 2 years ago

Is #782 needed anymore? It looks like all of those changes are here.

keflavich commented 2 years ago

This is branched off of #782; that should be merged first

codecov-commenter commented 2 years ago

Codecov Report

Merging #783 (c2f4373) into master (9b15939) will decrease coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #783      +/-   ##
==========================================
- Coverage   77.91%   77.90%   -0.01%     
==========================================
  Files          24       24              
  Lines        5818     5826       +8     
==========================================
+ Hits         4533     4539       +6     
- Misses       1285     1287       +2     
Impacted Files Coverage Δ
spectral_cube/spectral_cube.py 76.74% <100.00%> (+0.05%) :arrow_up:
spectral_cube/dask_spectral_cube.py 85.64% <0.00%> (-0.18%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9b15939...c2f4373. Read the comment docs.

keflavich commented 2 years ago

The commit history on this is a bit messed up right now; it's overlapped with #782 , which I'd like to resolve first; I'll have to rebase against that