Closed dgegen closed 4 months ago
Hi @dgegen and thanks for the changes, both relevant. astropy
is imported but astropy.units
is actually not used. That's intentional, to avoid having it as a dependency. Could you remove the import and keep astropy
out of the dependencies? Otherwise good catch on the new jax version.
Thanks!
Regarding astropy
: I'm afraid I've run into problems without adding astropy
as an additional dependency: If I install Nuance from GitHub into a fresh environment and then only change the jax.config
lines in the __init__.py
file, I face the following error:
>>> import nuance
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/usr/pydev/nuance/nuance/nuance/__init__.py", line 11, in <module>
from nuance.star import Star
File "/Users/usr/pydev/nuance/nuance/nuance/star.py", line 3, in <module>
import astropy.units as u
ModuleNotFoundError: No module named 'astropy'
Sure, it makes sense since it is imported in this file. Unless I am missing something I think you can remove the import in star.py
(https://github.com/lgrcia/nuance/blob/main/nuance/star.py#L3)
Oh my – I am being silly again! For some reason I had thought you just wanted me to take out the dependency, but re-reading it now, I see that you clearly said to take out the import as well 🙈 Sorry for the inconvenience – writing your last message must have been a bit disturbing :))
No worries at all :) Thanks a lot for these additions!
Oh one last thing before merging. Can you bump the minor version of the package in the pyproject file? It should be 0.6.1
now I think.
Done!
Two small changes to ensure that nuance runs without errors after installation via
pip
.from jax.config import config
raises anImportError
in the newest version of JAX (0.4.25). It turns out that importing thejax.config
submodule viaimport jax.config
is deprecated and referencing the config object viajax.config
is encouraged. Thus,config = jax.config
was added to__init__.py
, so thatnuance.config
can still be used to accessjax.config
as was the case previously, although one could probably debate whether this is desirable conceptually.star.py
importsimport astropy.units as u
, but astropy was not listed as a dependency inpyproject.toml
.Keep up the inspiring work!