BUG FIX for merged pull request #2762

[MichaelWolloch]

There was a bug in PR #2762 when using sha256 hashes for multi-element POTCARs which completely breaks their use!

• Fix 1: Changed regex when reading a POTCAR from a file so that the newline symbol (\n) is included in the last line.
• Fix 2: Added CPYR parsing again to avoid some warnings
• Feature 1: Added a unit test that reads a two-element POTCAR, where one has a sha256 hash and the other has not. validates both hashes.

Sorry for not completely testing this with actual workflows!

[coveralls]

Coverage decreased (-25.4%) to 19.412% when pulling f4d4abc on MichaelWolloch:master into 3b5b90e on materialsproject:master.

[MichaelWolloch]

I have no idea why so many test fail, but I think that these must be unrelated? pytest pymatgen/io/vasp/tests/ completes for me without errors.

SKIPPED [1] pymatgen/io/vasp/tests/test_outputs.py:1692: h5py required for HDF5 support.
241 passed, 1 skipped in 144.37s (0:02:24)

[mkhorton]

Thanks @MichaelWolloch, no worries, these things happen! We haven't released yet so still time to address :)

[mkhorton]

Frustrating the tests are failing, this will require some investigation; I think a mypy update has caught previously-un-caught issues too.

I agree the failures are unrelated to this PR; I will go ahead and merge since this is critical.

[MichaelWolloch]

Thanks @mkhorton for addressing this quickly and good luck with figuring out what is happening with all those failed tests.

[janosh]

@MichaelWolloch Just to confirm, the test file path here had a typo? If yes, I fixed it in e3bcad7 and there's nothing more to do.

- PymatgenTest.TEST_FILES_DIR / "POT_GGA_PAW_PBE_54" / "POTCAR.Fe_O.gz"
+ PymatgenTest.TEST_FILES_DIR / "POT_GGA_PAW_PBE_54" / "POTCAR.O.gz"

[MichaelWolloch]

Hi @janosh,

thanks for spotting the problem, although your fix was not quite right. I actually forgot to add the correct file (POTCAR.Fe_O.gz) to my commit. The corresponding unit test should check a POTCAR combined of two PotcarSingles (Fe and O) where one has a sha256 hash in the header, and one does not (See Feature 1 in the decription of this PR). This was not originally checked, and thus I did not realize that without this test in the initial PR about the hashes here.

Locally the tests passed for me, because I had the file in the correct folder, but I forgot to add it to the commit. Then I think other tests failed before that for unrelated reasons, so I did not spot the missing file error then.

I was on holiday until today, so I did not notice your comment 3 days ago until now. I think I have to make a new PR to upload the new file and change back the file loading line?

Cheers, Michael

[janosh]

Yes, if you could make a PR to add the file, that would be great!

[MichaelWolloch]

Pull request is done: #2796

Currently waiting for the tests to complete.