search

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

MNT: modernize yt usage #793

Closed neutrinoceros closed 2 years ago

neutrinoceros commented 2 years ago

content:

rationale: the yt.mods namespace is targetted for deprecation https://github.com/yt-project/yt/pull/3278 While I'm at it I fixed/updated a couple links to yt docs

codecov-commenter commented 2 years ago

Codecov Report

Merging #793 (f2deb4d) into master (7452dce) will not change coverage. The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #793   +/-   ##
=======================================
  Coverage   77.90%   77.90%           
=======================================
  Files          24       24           
  Lines        5826     5826           
=======================================
  Hits         4539     4539           
  Misses       1287     1287
Impacted Files Coverage Δ
spectral_cube/spectral_cube.py 76.74% <0.00%> (ø)

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 a5ec360...f2deb4d. Read the comment docs.

e-koch commented 2 years ago

I'll merge here. There may be a few more issues in getting spectral-cube's yt code working (#794) to be addressed in another PR.

neutrinoceros commented 2 years ago

Yes. Actually it looks like yt integration was written around the time yt 3.0 came around, and there are some version checks to determine wether yt 2.x or >3 is installed. The 2.x branch is long gone and the 3.x one isn't supported anymore (last release came out in 2019), and both none of these branches are explicitly compatible with most recent versions of Python (3.9 and 3.10). I doubt they even work now and it's likely not actually tested here. May I suggest to require yt>4.0 in spectral-cube so we can simplify the integration ?

keflavich commented 2 years ago

Yes, that would be fine!

neutrinoceros commented 2 years ago

Dully noted. I'll write a patch for this once #794 is fixed.