pyiron / pyiron_atomistics

pyiron_atomistics - an integrated development environment (IDE) for atomistic simulation in computational materials science.
https://pyiron-atomistics.readthedocs.io
BSD 3-Clause "New" or "Revised" License
44 stars 15 forks source link

Do not access files accidentely in interactive output #1348

Closed pmrv closed 4 months ago

pmrv commented 8 months ago

Previously we indexed into the job itself, but that also triggers a search for (possibly) compressed files and is slow. Since we only intend to look into the HDF anyway, we just do it directly now.

May fix https://github.com/pyiron/pyiron_base/issues/1347

coveralls commented 8 months ago

Pull Request Test Coverage Report for Build 9415515695

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
pyiron_atomistics/sphinx/interactive.py 3 5 60.0%
pyiron_atomistics/atomistics/job/atomistic.py 39 47 82.98%
<!-- Total: 46 56 82.14% -->
Files with Coverage Reduction New Missed Lines %
pyiron_atomistics/atomistics/job/atomistic.py 4 76.94%
<!-- Total: 4 -->
Totals Coverage Status
Change from base Build 9407382474: 0.05%
Covered Lines: 10677
Relevant Lines: 15015

💛 - Coveralls
pmrv commented 5 months ago

Fine by me as long as integration tests pass

Mh, seems this is actually broken on main, as it also pop up in #1426. I cannot actually run this test locally (hangs indefinitely), do you have an idea?

samwaseda commented 5 months ago

Yeah it looks like it's a Sphinx problem right? Somehow it doesn't appear in my environment...

samwaseda commented 5 months ago

I just checked everywhere but I just cannot reproduce the error...

samwaseda commented 5 months ago

Ah now I updated the conda environment and it started appearing. Now I'll look into it.

samwaseda commented 5 months ago

Now as this PR got merged hopefully the tests will pass :)