Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test monty fix for reverse_readline #4068

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ jobs:

uv pip install --editable '.[${{ matrix.config.extras }}]' --resolution=${{ matrix.config.resolution }}

# TODO: test monty fix for reverse readline
uv pip install git+https://github.com/DanielYang59/monty.git@readline-line-ending

- name: Install optional Ubuntu dependencies
if: matrix.config.os == 'ubuntu-latest'
run: |
Expand Down
2 changes: 1 addition & 1 deletion dev_scripts/potcar_scrambler.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
from typing import TYPE_CHECKING

import numpy as np
from monty.io import zopen
from monty.os.path import zpath
from monty.serialization import zopen

from pymatgen.core import SETTINGS
from pymatgen.io.vasp import Potcar, PotcarSingle
Expand Down
22 changes: 10 additions & 12 deletions src/pymatgen/io/adf.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
import re
from typing import TYPE_CHECKING

from monty.io import reverse_readline
from monty.io import reverse_readfile
from monty.itertools import chunks
from monty.json import MSONable
from monty.serialization import zopen

from pymatgen.core.structure import Molecule

Expand Down Expand Up @@ -645,16 +644,15 @@ def _parse_logfile(self, logfile):
# The last non-empty line of the logfile must match the end pattern.
# Otherwise the job has some internal failure. The TAPE13 part of the
# ADF manual has a detailed explanation.
with zopen(logfile, mode="rt") as file:
for line in reverse_readline(file):
if line == "":
continue
if end_patt.search(line) is None:
self.is_internal_crash = True
self.error = "Internal crash. TAPE13 is generated!"
self.is_failed = True
return
break
for line in reverse_readfile(logfile):
if line.strip() == "":
continue
if end_patt.search(line) is None:
self.is_internal_crash = True
self.error = "Internal crash. TAPE13 is generated!"
self.is_failed = True
return
break

with open(logfile) as file:
for line in file:
Expand Down
3 changes: 2 additions & 1 deletion tests/core/test_tensors.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

import numpy as np
import pytest
from monty.serialization import MontyDecoder, loadfn
from monty.json import MontyDecoder
from monty.serialization import loadfn
from numpy.testing import assert_allclose
from pytest import approx

Expand Down
2 changes: 1 addition & 1 deletion tests/io/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from typing import TYPE_CHECKING

import pytest
from monty.serialization import MontyDecoder
from monty.json import MontyDecoder

from pymatgen.core.structure import Structure
from pymatgen.io.cif import CifParser, CifWriter
Expand Down
19 changes: 16 additions & 3 deletions tests/io/vasp/test_outputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import numpy as np
import pytest
from monty.io import zopen
from monty.shutil import decompress_file
from monty.tempfile import ScratchDir
from numpy.testing import assert_allclose
from pytest import approx

Expand Down Expand Up @@ -832,9 +834,20 @@ def test_parse_potcar_cwd_relative(self):
assert vrun.potcar_spec[ipot]["summary_stats"] == potcar[ipot]._summary_stats


class TestOutcar(PymatgenTest):
Copy link
Contributor Author

@DanielYang59 DanielYang59 Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to avoid inheriting from PymatgenTest as TestCase (and its subclasses) currently don't support parametrize, I believe this wouldn't affect this test as no PymatgenTest/TestCase specific method is used here.

A quick recreate:

from unittest import TestCase

import pytest


class TestDemo(TestCase):  # `class TestDemo` works just fine
    @pytest.mark.parametrize("string", ["hello", "world"])
    def test_hello(self, string):
        print(string)
        pytest.fail("should fail, test")

Would give:

====================================================== test session starts =======================================================
platform darwin -- Python 3.12.7, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/yang/developer/test
plugins: anyio-4.6.2.post1
collected 1 item                                                                                                                 

test_pytest.py F                                                                                                           [100%]

============================================================ FAILURES ============================================================
______________________________________________________ TestDemo.test_hello _______________________________________________________

self = <unittest.case._Outcome object at 0x103480950>, test_case = <test_pytest.TestDemo testMethod=test_hello>, subTest = False

    @contextlib.contextmanager
    def testPartExecutor(self, test_case, subTest=False):
        old_success = self.success
        self.success = True
        try:
>           yield

/opt/homebrew/Cellar/[email protected]/3.12.7_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py:58: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/opt/homebrew/Cellar/[email protected]/3.12.7_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py:634: in run
    self._callTestMethod(testMethod)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_pytest.TestDemo testMethod=test_hello>
method = <bound method TestDemo.test_hello of <test_pytest.TestDemo testMethod=test_hello>>

    def _callTestMethod(self, method):
>       if method() is not None:
E       TypeError: TestDemo.test_hello() missing 1 required positional argument: 'string'

/opt/homebrew/Cellar/[email protected]/3.12.7_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py:589: TypeError
==================================================== short test summary info =====================================================
FAILED test_pytest.py::TestDemo::test_hello - TypeError: TestDemo.test_hello() missing 1 required positional argument: 'string'
======================================================= 1 failed in 0.04s ========================================================

def test_init(self):
outcar = Outcar(f"{VASP_OUT_DIR}/OUTCAR.gz")
class TestOutcar:
@pytest.mark.parametrize("compressed", [True, False])
def test_init(self, compressed: bool):
"""Test from both compressed and uncompressed versions,
as there was a bug in monty causing different behaviours.
"""
with ScratchDir("."):
if compressed:
copyfile(f"{VASP_OUT_DIR}/OUTCAR.gz", "./OUTCAR.gz")
decompress_file("./OUTCAR.gz")
outcar = Outcar("./OUTCAR")
else:
outcar = Outcar(f"{VASP_OUT_DIR}/OUTCAR.gz")

expected_mag = (
{"d": 0.0, "p": 0.003, "s": 0.002, "tot": 0.005},
{"d": 0.798, "p": 0.008, "s": 0.007, "tot": 0.813},
Expand Down