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

VaspInputSet couldn't write cif structures #2721

Merged
merged 8 commits into from
Nov 8, 2022

Conversation

JiQi535
Copy link
Contributor

@JiQi535 JiQi535 commented Nov 7, 2022

Summary

This pr deals with the failure of writing .cif format structures by VaspInputSet. I encountered an error of 'PosixPath' object has no attribute 'lower' when using the write_input method with include_cif=True. After casting the PosixPath to str, this problem is solved.

@shyuep
Copy link
Member

shyuep commented Nov 8, 2022

Pls add a test for this.

@coveralls
Copy link

coveralls commented Nov 8, 2022

Coverage Status

Coverage remained the same at 0.0% when pulling 10afe19 on JiQi535:master into ffbebce on materialsproject:master.

@janosh
Copy link
Member

janosh commented Nov 8, 2022

@shyuep There is already a test for this actually.

self.mitset.write_input(".", make_dir_if_not_present=True, include_cif=True)

The tests are just not running because of

if SETTINGS.get("PMG_VASP_PSP_DIR") is None:
raise unittest.SkipTest(f"PMG_VASP_PSP_DIR is not set. Skipping tests in {__file__}.")

We're not setting PMG_VASP_PSP_DIR. This is not due to #2708 btw, looks like they weren't running before either.

When I run

export PMG_VASP_PSP_DIR=/path/to/vasp/pseudo-potentials
pytest pymatgen/io/vasp/tests/test_sets.py -k test_write_input

I can reproduce the error @JiQi535 reported:

>       if fmt == "cif" or fnmatch(filename.lower(), "*.cif*"):
E       AttributeError: 'PosixPath' object has no attribute 'lower'

@janosh janosh enabled auto-merge (squash) November 8, 2022 04:21
@janosh janosh merged commit d17a0f4 into materialsproject:master Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants