pybamm-team / PyBaMM

Fast and flexible physics-based battery models in Python
https://www.pybamm.org/
BSD 3-Clause "New" or "Revised" License
1.11k stars 546 forks source link

[Bug]: v24.9.0 broken with [cite] optional dependency #4432

Closed BradyPlanden closed 1 month ago

BradyPlanden commented 1 month ago

PyBaMM Version

v24.9.0

Python Version

3.12

Describe the bug

Installing with optional [cite] dependency results with the below import error.

Steps to Reproduce

python -m pip install 'pybamm[cite]' produces the below error when running import pybamm.

The above exception was the direct cause of the following exception:

ModuleNotFoundError                       Traceback (most recent call last)
Cell In[1], line 1
----> 1 import pybamm

File ~/.pyenv/versions/3.12.4/envs/pybamm-cite-test/lib/python3.12/site-packages/pybamm/__init__.py:25
     23 from .logger import logger, set_logging_level, get_new_logger
     24 from .settings import settings
---> 25 from .citations import Citations, citations, print_citations
     27 # Classes for the Expression Tree
     28 from .expression_tree.symbol import *

File ~/.pyenv/versions/3.12.4/envs/pybamm-cite-test/lib/python3.12/site-packages/pybamm/citations.py:271
    265         raise Exception(
    266             "Verbose output is available only for the terminal and not for printing to files",
    267         )
    268     pybamm.citations.print(filename, output_format, verbose)
--> 271 citations = Citations()

File ~/.pyenv/versions/3.12.4/envs/pybamm-cite-test/lib/python3.12/site-packages/pybamm/citations.py:36, in Citations.__init__(self)
     33 # Dict mapping citations keys to BibTex entries
     34 self._all_citations: dict[str, str] = dict()
---> 36 self.read_citations()
     37 self._reset()

File ~/.pyenv/versions/3.12.4/envs/pybamm-cite-test/lib/python3.12/site-packages/pybamm/citations.py:68, in Citations.read_citations(self)
     64 """Reads the citations in `pybamm.CITATIONS.bib`. Other works can be cited
     65 by passing a BibTeX citation to :meth:`register`.
     66 """
     67 if not self._module_import_error:
---> 68     parse_file = import_optional_dependency("pybtex.database", "parse_file")
     69     citations_file = os.path.join(pybamm.__path__[0], "CITATIONS.bib")
     70     bib_data = parse_file(citations_file, bib_format="bibtex")

File ~/.pyenv/versions/3.12.4/envs/pybamm-cite-test/lib/python3.12/site-packages/pybamm/util.py:332, in import_optional_dependency(module_name, attribute)
    328         return module
    330 except ModuleNotFoundError as error:
    331     # Raise an ModuleNotFoundError if the module or attribute is not available
--> 332     raise ModuleNotFoundError(err_msg) from error

ModuleNotFoundError: Optional dependency pybtex.database is not available. See https://docs.pybamm.org/en/latest/source/user_guide/installation/index.html#optional-dependencies for more details.

Relevant log output

No response

agriyakhetarpal commented 1 month ago

cc: @kratman who was working on this back in #4383. I don't know why this fails, though, because we test against this behaviour as well...

kratman commented 1 month ago

Yeah I am looking into it

agriyakhetarpal commented 1 month ago

Thanks! I guess we should release 24.9.1 with a fix by today? Also, we install all the optional dependencies when testing the wheels, too, which includes [cite], so this shouldn't have happened.

kratman commented 1 month ago

@BradyPlanden There must be a dependency issue, I can reproduce this with

pip install 'pybamm[cite]'

but not with

pip install 'pybamm[all,cite]'
agriyakhetarpal commented 1 month ago

I'm unable to reproduce with pybamm[cite]

agriyakhetarpal commented 1 month ago

This is what I tried:

mamba create -n pybamm-test-env python
mamba activate pybamm-test-env
python -m pip install 'pybamm[cite]'
python -c "import pybamm"  # works

Edit: and couldn't reproduce it for a second time

agriyakhetarpal commented 1 month ago
/opt/homebrew/bin/python3.12 -m venv venv
source venv/bin/activate
python -m pip install pybamm"[cite]"   # this fails
agriyakhetarpal commented 1 month ago

@kratman and @BradyPlanden, this is coming from an undeclared dependency on setuptools, we require it on Python 3.12 and later because pybtex requires it. Virtual environments for Python 3.12 and later don't include seed packages (such as setuptools and wheel – mamba envs do, which is why I couldn't reproduce in the earlier example). Installing setuptools in the venv above fixes the error.

agriyakhetarpal commented 1 month ago

Unfortunately, pybtex is still unmaintained and they haven't released the fix for this yet (see #3648).

kratman commented 1 month ago

The reason you do not see it with pip install "pybamm[all]" is because of pip install "pybamm[examples]".

BradyPlanden commented 1 month ago

It looks like pybtex has a unreleased fix. It's a bit annoying that this is due to not deploying a release. Here's the relevant issue: https://bitbucket.org/pybtex-devs/pybtex/issues/169/replace-pkg_resources-with

agriyakhetarpal commented 1 month ago

Yes, this is why we were installing setuptools in our nox sessions

kratman commented 1 month ago

Yeah we just need to swap out pybtex and make citations non-optional. The citations object is global, all the "check if it is installed" stuff is because we are hiding a dependency too

agriyakhetarpal commented 1 month ago

I would be okay with making citations non-optional, too. I think the functionality is nice to have by default, since we encourage researchers to use pybamm.print_citations() at the end of each script, especially through the example notebooks.