Add unit test for io.vasp.help (#4020)

* enable TLS cert verify

* use https over http

* explicitly declare bs4

* add bs4 as opt dep

* reduce timeout in CI to 60 secs

* add test for io.vasp.help

* add test for get_incar_tags

* declare html parser

* reduce test timeout to 5 sec

* use pytest importorskip

* also except timeout and assert incar tags as list

* bump python to 3.10+ in readme, thanks @QuantumChemist

* update `test_get_incar_tags` to check incar_parameters.json

* skip init for cls method test

* add missing VASP wiki INCAR page

* include length in error msg

* fix typo

* update doc py 3.9 to 3.10

* reduce requests timeout to 60 secs, 600 sec might be impractical

* make comment docstring

* revert to just assert incar_tags_wiki

* assert some hard coded INCAR tags

CONTRIBUTING.md

@@ -78,7 +78,7 @@ Given that `pymatgen` is intended to be a long-term code base, we adopt very str
     pre-commit run --all-files  # ensure your entire codebase passes linters
    ```
 
-1. **Python 3**. We only support Python 3.9+.
+1. **Python 3**. We only support Python 3.10+.
 1. **Documentation** is required for all modules, classes and methods. In particular, the method doc strings should make clear the arguments expected and the return values. For complex algorithms (e.g., an Ewald summation), a summary of the algorithm should be provided and preferably with a link to a publication outlining the method in detail.
 
 For the above, if in doubt, please refer to the core classes in `pymatgen` for examples of what is expected.

README.md

@@ -12,7 +12,7 @@ height="70">
 [![codecov](https://codecov.io/gh/materialsproject/pymatgen/branch/master/graph/badge.svg?token=XC47Un1LV2)](https://codecov.io/gh/materialsproject/pymatgen)
 [![PyPI Downloads](https://img.shields.io/pypi/dm/pymatgen?logo=pypi&logoColor=white&color=blue&label=PyPI)](https://pypi.org/project/pymatgen)
 [![Conda Downloads](https://img.shields.io/conda/dn/conda-forge/pymatgen?logo=condaforge&color=blue&label=Conda)](https://anaconda.org/conda-forge/pymatgen)
-[![Requires Python 3.9+](https://img.shields.io/badge/Python-3.9+-blue.svg?logo=python&logoColor=white)](https://python.org/downloads)
+[![Requires Python 3.10+](https://img.shields.io/badge/Python-3.10+-blue.svg?logo=python&logoColor=white)](https://python.org/downloads)
 [![Paper](https://img.shields.io/badge/J.ComMatSci-2012.10.028-blue?logo=elsevier&logoColor=white)](https://doi.org/10.1016/j.commatsci.2012.10.028)
 
 </h4>
@@ -63,7 +63,7 @@ If you'd like to use the latest unreleased changes on the main branch, you can i
 pip install -U git+https://github.com/materialsproject/pymatgen
 ```
 
-The minimum Python version is 3.9. Some extra functionality (e.g., generation of POTCARs) does require additional setup (see the [`pymatgen` docs]).
+The minimum Python version is 3.10. Some extra functionality (e.g., generation of POTCARs) does require additional setup (see the [`pymatgen` docs]).
 
 ## Change Log
 

dev_scripts/update_pt_data.py

@@ -239,7 +239,7 @@ def gen_iupac_ordering():
 def add_electron_affinities():
     """Update the periodic table data file with electron affinities."""
 
-    req = requests.get("https://wikipedia.org/wiki/Electron_affinity_(data_page)", timeout=600)
+    req = requests.get("https://wikipedia.org/wiki/Electron_affinity_(data_page)", timeout=60)
     soup = BeautifulSoup(req.text, "html.parser")
     table = None
     for table in soup.find_all("table"):

pyproject.toml

@@ -99,6 +99,7 @@ ci = ["pytest-cov>=4", "pytest-split>=0.8", "pytest>=8"]
 docs = ["invoke", "sphinx", "sphinx_rtd_theme", "sphinx_markdown_builder"]
 optional = [
     "ase>=3.23.0",
+    "beautifulsoup4",
     # TODO: uv cannot install BoltzTraP2
     # "BoltzTraP2>=24.7.2 ; platform_system != 'Windows'",
     "chemview>=0.6",

src/pymatgen/ext/cod.py

@@ -112,7 +112,7 @@ def get_structure_by_formula(self, formula: str, **kwargs) -> list[dict[str, str
         for line in text:
             if line.strip():
                 cod_id, sg = line.split("\t")
-                response = requests.get(f"https://{self.url}/cod/{cod_id.strip()}.cif", timeout=600)
+                response = requests.get(f"https://{self.url}/cod/{cod_id.strip()}.cif", timeout=60)
                 try:
                     struct = Structure.from_str(response.text, fmt="cif", **kwargs)
                     structures.append({"structure": struct, "cod_id": int(cod_id), "sg": sg})

src/pymatgen/ext/matproj_legacy.py

@@ -1570,7 +1570,7 @@ def _check_get_download_info_url_by_task_id(self, prefix, task_ids) -> list[str]
 
     @staticmethod
     def _check_nomad_exist(url) -> bool:
-        response = requests.get(url=url, timeout=600)
+        response = requests.get(url=url, timeout=60)
         if response.status_code != 200:
             return False
         content = json.loads(response.text)

src/pymatgen/io/core.py

@@ -178,7 +178,7 @@ def __delitem__(self, key: PathLike) -> None:
         del self.inputs[key]
 
     def __or__(self, other: dict | Self) -> Self:
-        # enable dict merge operator | for InputSet
+        """Enable dict merge operator |."""
         if isinstance(other, dict):
             other = type(self)(other)
         if not isinstance(other, type(self)):

src/pymatgen/io/vasp/help.py

@@ -16,7 +16,7 @@
 class VaspDoc:
     """A VASP documentation helper."""
 
-    @requires(BeautifulSoup, "BeautifulSoup must be installed to fetch from the VASP wiki.")
+    @requires(BeautifulSoup, "BeautifulSoup4 must be installed to fetch from the VASP wiki.")
     def __init__(self) -> None:
         """Init for VaspDoc."""
         self.url_template = "https://www.vasp.at/wiki/index.php/%s"
@@ -55,10 +55,9 @@ def get_help(cls, tag: str, fmt: str = "text") -> str:
         tag = tag.upper()
         response = requests.get(
             f"https://www.vasp.at/wiki/index.php/{tag}",
-            verify=False,  # noqa: S501
-            timeout=600,
+            timeout=60,
         )
-        soup = BeautifulSoup(response.text)
+        soup = BeautifulSoup(response.text, features="html.parser")
         main_doc = soup.find(id="mw-content-text")
         if fmt == "text":
             output = main_doc.text
@@ -70,12 +69,13 @@ def get_help(cls, tag: str, fmt: str = "text") -> str:
     def get_incar_tags(cls) -> list[str]:
         """Get a list of all INCAR tags from the VASP wiki."""
         tags = []
-        for page in (
-            "https://www.vasp.at/wiki/index.php/Category:INCAR",
-            "https://www.vasp.at/wiki/index.php?title=Category:INCAR&pagefrom=ML+FF+LCONF+DISCARD#mw-pages",
+        for url in (
+            "https://www.vasp.at/wiki/index.php/Category:INCAR_tag",
+            "https://www.vasp.at/wiki/index.php?title=Category:INCAR_tag&pagefrom=LREAL#mw-pages",
+            "https://www.vasp.at/wiki/index.php?title=Category:INCAR_tag&pagefrom=Profiling#mw-pages",
         ):
-            response = requests.get(page, verify=False, timeout=600)  # noqa: S501
-            soup = BeautifulSoup(response.text)
+            response = requests.get(url, timeout=60)
+            soup = BeautifulSoup(response.text, features="html.parser")
             for div in soup.findAll("div", {"class": "mw-category-group"}):
                 children = div.findChildren("li")
                 for child in children:

tasks.py

@@ -134,7 +134,7 @@ def release_github(ctx: Context, version: str) -> None:
         "https://api.github.com/repos/materialsproject/pymatgen/releases",
         data=json.dumps(payload),
         headers={"Authorization": f"token {os.environ['GITHUB_RELEASES_TOKEN']}"},
-        timeout=600,
+        timeout=60,
     )
     print(response.text)
 
@@ -160,7 +160,7 @@ def update_changelog(ctx: Context, version: str | None = None, dry_run: bool = F
             pr_number = re_match[1]
             contributor, pr_name = re_match[2].split("/", 1)
             response = requests.get(
-                f"https://api.github.com/repos/materialsproject/pymatgen/pulls/{pr_number}", timeout=600
+                f"https://api.github.com/repos/materialsproject/pymatgen/pulls/{pr_number}", timeout=60
             )
             lines += [f"* PR #{pr_number} from @{contributor} {pr_name}"]
             json_resp = response.json()

tests/core/test_structure.py

@@ -938,7 +938,7 @@ def test_from_id(self):
         assert s.reduced_formula == "Al2O3"
 
         try:
-            website_down = requests.get("https://www.crystallography.net", timeout=600).status_code != 200
+            website_down = requests.get("https://www.crystallography.net", timeout=60).status_code != 200
         except (requests.exceptions.ConnectionError, urllib3.exceptions.ConnectTimeoutError):
             website_down = True
         if not website_down:

tests/ext/test_cod.py

@@ -15,7 +15,7 @@
     pytest.skip(allow_module_level=True, reason="Skip COD test in CI")
 
 try:
-    website_down = requests.get("https://www.crystallography.net", timeout=600).status_code != 200
+    website_down = requests.get("https://www.crystallography.net", timeout=60).status_code != 200
 except (requests.exceptions.ConnectionError, urllib3.exceptions.ConnectTimeoutError):
     website_down = True
 

tests/ext/test_matproj.py

@@ -33,7 +33,7 @@
 else:
     MP_URL = "https://materialsproject.org"
 try:
-    skip_mprester_tests = requests.get(MP_URL, timeout=600).status_code != 200
+    skip_mprester_tests = requests.get(MP_URL, timeout=60).status_code != 200
 
 except (ModuleNotFoundError, ImportError, requests.exceptions.ConnectionError):
     # Skip all MPRester tests if some downstream problem on the website, mp-api or whatever.
@@ -81,7 +81,7 @@ def test_get_data(self):
             "total_magnetization",
         }
         mp_id = "mp-1143"
-        vals = requests.get(f"https://legacy.materialsproject.org/materials/{mp_id}/json/", timeout=600)
+        vals = requests.get(f"https://legacy.materialsproject.org/materials/{mp_id}/json/", timeout=60)
         expected_vals = vals.json()
 
         for prop in props:
@@ -575,7 +575,7 @@ def test_get_data(self):
             "total_magnetization",
         }
         mp_id = "mp-1143"
-        vals = requests.get(f"https://legacy.materialsproject.org/materials/{mp_id}/json/", timeout=600)
+        vals = requests.get(f"https://legacy.materialsproject.org/materials/{mp_id}/json/", timeout=60)
         expected_vals = vals.json()
 
         for prop in props:

tests/ext/test_optimade.py

@@ -13,12 +13,12 @@
     website_down = True
 
 try:
-    optimade_providers_down = requests.get("https://providers.optimade.org", timeout=600).status_code not in (200, 403)
+    optimade_providers_down = requests.get("https://providers.optimade.org", timeout=60).status_code not in (200, 403)
 except requests.exceptions.ConnectionError:
     optimade_providers_down = True
 
 try:
-    mc3d_down = requests.get(OptimadeRester.aliases["mcloud.mc3d"] + "/v1/info", timeout=600).status_code not in (
+    mc3d_down = requests.get(OptimadeRester.aliases["mcloud.mc3d"] + "/v1/info", timeout=60).status_code not in (
         200,
         403,
         301,
@@ -27,7 +27,7 @@
     mc3d_down = True
 
 try:
-    mc2d_down = requests.get(OptimadeRester.aliases["mcloud.mc2d"] + "/v1/info", timeout=600).status_code not in (
+    mc2d_down = requests.get(OptimadeRester.aliases["mcloud.mc2d"] + "/v1/info", timeout=60).status_code not in (
         200,
         403,
         301,

tests/io/vasp/test_help.py

@@ -0,0 +1,44 @@
+from __future__ import annotations
+
+import io
+from unittest.mock import patch
+
+import pytest
+import requests
+
+from pymatgen.io.vasp.help import VaspDoc
+
+BeautifulSoup = pytest.importorskip("bs4").BeautifulSoup
+
+
+try:
+    website_down = requests.get("https://www.vasp.at", timeout=5).status_code != 200
+except (requests.exceptions.ConnectionError, requests.exceptions.ReadTimeout):
+    website_down = True
+
+
+@pytest.mark.skipif(website_down, reason="www.vasp.at is down")
+class TestVaspDoc:
+    @pytest.mark.parametrize("tag", ["ISYM"])
+    def test_print_help(self, tag):
+        vasp_doc = VaspDoc()
+        with patch("sys.stdout", new=io.StringIO()) as fake_stdout:
+            vasp_doc.print_help(tag)
+            output = fake_stdout.getvalue()
+
+        assert tag in output
+
+    @pytest.mark.parametrize("tag", ["ISYM"])
+    def test_get_help(self, tag):
+        docstr = VaspDoc.get_help(tag)
+
+        assert tag in docstr
+
+    def test_get_incar_tags(self):
+        """Get all INCAR tags and check incar_parameters.json file."""
+        incar_tags_wiki = VaspDoc.get_incar_tags()
+        assert isinstance(incar_tags_wiki, list)
+
+        known_incar_tags = ("ENCUT", "ISMEAR")
+        for tag in known_incar_tags:
+            assert tag in incar_tags_wiki