Atomistics causing downstream tests to fail because periodic table can't be found
liamhuber opened this issue · 7 comments
In pyiron_workflow and pyiron_contrib we now get errors ending like:
File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_atomistics/atomistics/structure/factory.py", line 167, in bulk
return self.ase.bulk(
^^^^^^^^^^^^^^
File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_atomistics/atomistics/structure/factories/ase.py", line 49, in wrapper
return func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^
File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_atomistics/atomistics/structure/factories/ase.py", line 60, in bulk
return ase_to_pyiron(ase_bulk(*args, **kwargs))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_atomistics/atomistics/structure/atoms.py", line 3111, in ase_to_pyiron
pyiron_atoms = Atoms(
^^^^^^
File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_atomistics/atomistics/structure/atoms.py", line 122, in __init__
self._pse = PeriodicTable()
^^^^^^^^^^^^^^^
File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_atomistics/atomistics/structure/periodic_table.py", line 218, in __init__
self.dataframe = self._get_periodic_table_df(file_name)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_atomistics/atomistics/structure/periodic_table.py", line 419, in _get_periodic_table_df
raise ValueError("Was not able to locate a periodic table. ")
ValueError: Was not able to locate a periodic table.
I have a meeting now, but will poke into it after...
Presumably we have to install pyiron-data separately, as this is currently not available on pypi and therefore no longer included in the pyiron_atomistics package.
Testing this hypothesis in pyiron/pyiron_contrib#886 and pyiron/pyiron_workflow#20
Testing this hypothesis in pyiron/pyiron_contrib#886 and pyiron/pyiron_workflow#20
Good call, nice pragmatic attack.
The push-pull tests all passed, so I added the run_coverage label on contrib to redo the exact test that failed on the daily workflow.
It sure looks to me like we need the data dependency here in pyiron_atomistics.
The push-pull tests all passed, so I added the run_coverage label on contrib to redo the exact test that failed on the daily workflow.
Ah, looks like I actually wanted codeQL. Anyhow, threw that label on too now.
So the problem in the downstream repos makes sense, and the solution is easy (add pyiron-data as a dependency here). What I don't understand is how the pyiron_atomistics unit tests passed?
If pyiron-data appeared in the conda environment hierarchy, then the downstream repos would have gotten it installed as well, and it certainly doesn't appear in .ci_support/environment.yml directly. It does appear in .ci_support/environment_notebooks.yml and .binder/environment.yml, but these are not what's being used for the unit tests -- .ci_support/environment.yml is. The only other thing that makes sense to me is that setup.py is pulling it in when the unit tests do pip install, but I don't see anything there that would provide it either.
In pyiron_atomistics the resources are included in /tests/static https://github.com/pyiron/pyiron_atomistics/tree/main/tests/static
In
pyiron_atomisticsthe resources are included in/tests/statichttps://github.com/pyiron/pyiron_atomistics/tree/main/tests/static
Aha, true. We shouldn't do this. If instantiating our Atoms class requires pyiron-data (i.e. pyiron-resources) or an equivalent, then pyiron-data should be a dependency so that the library works out-of-the-box on downstream packages that depend on it. Also, then we don't need to reproduce the periodic table (and I don't know what else) in tests/static.