docs(formats): expand generated format reference#1044
Conversation
Document every registered format and conversion parameter, enrich generated reference pages with signatures and examples, and enforce coverage with Ruff and tests. Coding-Agent: Codex Codex-Version: codex-cli 0.144.6 Model: gpt-5.6-sol Reasoning-Effort: xhigh
dosubot
Bot
added
size:XXL
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1044 +/- ##
=======================================
Coverage 87.63% 87.63%
=======================================
Files 90 90
Lines 9209 9209
=======================================
Hits 8070 8070
Misses 1139 1139 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR strengthens dpdata’s generated “Supported Formats” documentation by making format pages more informative and consistent as plugins evolve, while also making the docs build easier to validate in typical contributor environments.
Changes:
- Added a documentation contract test that enforces class overviews and parameter documentation across registered format conversions.
- Reworked the docs generation script and supported-formats landing page to show preferred/equivalent aliases, clearer summaries, and copyable conversion examples.
- Improved/normalized many format/plugin docstrings and enabled Ruff D205 (blank line after docstring summary); made JupyterLite optional during regular Sphinx builds.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_format_docs.py | Adds contract tests to keep format docstrings/pages complete as plugins change. |
| pyproject.toml | Enables Ruff D205 by removing it from ignores. |
| dpdata/utils.py | Docstring refinements for utility helpers (notably open_file). |
| dpdata/system.py | Updates docstrings for core System/MultiSystems helpers to match stricter docstring linting. |
| dpdata/driver.py | Improves base-class docstrings for driver/minimizer plugins. |
| dpdata/plugins/xyz.py | Adds/expands format overviews and NumPy-style method docstrings for XYZ/extxyz aliases. |
| dpdata/plugins/vasp.py | Adds richer VASP format overviews and conversion docstrings (POSCAR/OUTCAR/XML). |
| dpdata/plugins/siesta.py | Adds SIESTA format overviews and method parameter docs. |
| dpdata/plugins/rdkit.py | Adds RDKit-backed format overviews and method docstrings for mol/sdf. |
| dpdata/plugins/qe.py | Adds Quantum ESPRESSO format overviews and method docstrings. |
| dpdata/plugins/pymatgen.py | Adds pymatgen adapter overviews and method parameter/return docs. |
| dpdata/plugins/pwmat.py | Adds PWmat format overviews and method docstrings. |
| dpdata/plugins/psi4.py | Clarifies PSI4 format docs; normalizes **kwargs documentation. |
| dpdata/plugins/orca.py | Clarifies ORCA output format docs; normalizes **kwargs documentation. |
| dpdata/plugins/openmx.py | Clarifies OpenMX format overview and usage guidance. |
| dpdata/plugins/n2p2.py | Expands n2p2 format overview with spec link and clearer description. |
| dpdata/plugins/list.py | Improves write-only list format overview and method docs. |
| dpdata/plugins/lammps.py | Adds/extends LAMMPS format overview and parameters docs (including **kwargs). |
| dpdata/plugins/gromacs.py | Improves GRO format overview and documents writer “Other Parameters”. |
| dpdata/plugins/gaussian.py | Adds Gaussian format overviews and method parameter docs. |
| dpdata/plugins/fhi_aims.py | Adds FHI-aims format overviews and method docstrings. |
| dpdata/plugins/dftbplus.py | Replaces verbose docstring with concise, accurate DFTB+ overview. |
| dpdata/plugins/deepmd.py | Adds detailed DeePMD format overviews and method docstrings; refines parameter docs. |
| dpdata/plugins/cp2k.py | Adds CP2K format overviews and method parameter docs. |
| dpdata/plugins/ase.py | Improves ASE adapter/trajectory overviews and method parameter docs. |
| dpdata/plugins/amber.py | Adds AMBER/SQM overviews and method docstrings for read/write parameters. |
| dpdata/plugins/abacus.py | Adds ABACUS format overviews and method parameter docs. |
| dpdata/plugins/3dmol.py | Clarifies py3Dmol adapter as write-only, in-memory visualization. |
| dpdata/formats/rdkit/sanitize.py | Cleans up docstring formatting artifact. |
| dpdata/formats/lmdb/format.py | Adds NumPy-style parameter documentation for LMDB conversions. |
| dpdata/formats/gaussian/fchk.py | Converts Google-style docstring to NumPy-style with clearer errors/returns. |
| dpdata/formats/amber/md.py | Improves function docstring summary formatting. |
| dpdata/formats/abacus/stru.py | Tightens parameter type documentation. |
| dpdata/formats/abacus/relax.py | Improves docstring summary/formatting and clarifies intent. |
| docs/make_format.py | Major rework of format page generation (preferred aliases, examples, stable output paths). |
| docs/formats.rst | Expands supported-formats landing page with data-model guidance and examples. |
| docs/conf.py | Makes JupyterLite optional via env var; fixes script invocation; updates language/intersphinx. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| file : file object or file path | ||
| A file object or a file path. | ||
| *args : list | ||
| Positional arguments passed to :func:`open` for path inputs. | ||
| **kwargs : dict | ||
| Keyword arguments passed to :func:`open` for path inputs. |
📝 WalkthroughWalkthroughThe PR expands format documentation, restructures generated format references, improves documentation build configuration, clarifies plugin APIs, removes the D205 lint exception, and adds tests enforcing format class and conversion-method documentation. ChangesDocumentation generation and user guide
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dpdata/plugins/deepmd.py (1)
492-507: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument both labeled and unlabeled consumers.
The supplied
dpdata/system.pycontract sends these groups tofrom_labeled_systemby default and tofrom_systemonly whenlabeled=False. The current wording is misleading for the default path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dpdata/plugins/deepmd.py` around lines 492 - 507, Update the HDF5 group generator docstring near from_labeled_system/from_system to document both consumers: groups are passed to from_labeled_system by default, and to from_system only when labeled=False. Correct the parameter or description text without changing runtime behavior.
🧹 Nitpick comments (1)
docs/make_format.py (1)
522-533: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
internal_parametersinstead of re-hardcoding the hidden names.This set duplicates
internal_parameters(Lines 154-159); the two can drift when a new dispatch-supplied parameter is added.♻️ Suggested refactor
if len(doc_obj["Parameters"]) > 0: + hidden = internal_parameters.get(method, set()) | {"*args"} doc_obj["Parameters"] = [ - xx - for xx in doc_obj["Parameters"] - if xx.name - not in { - "data", - "formulas", - "mol", - "rdkit_mol", - "*args", - } + xx for xx in doc_obj["Parameters"] if xx.name not in hidden ]Note this narrows filtering to the current method's own internal parameters; if a shared
to_systemimplementation is reused forto_bond_order_systemdocs, keep the union of the relevant entries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/make_format.py` around lines 522 - 533, Update the Parameters filtering in the documentation generation flow to reuse the existing internal_parameters definition instead of duplicating the hidden-name set. Ensure the filtering includes the current method’s internal parameters and preserves the union of relevant entries when a shared to_system implementation serves to_bond_order_system documentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dpdata/plugins/abacus.py`:
- Around line 31-46: Update the from_system docstring’s file_name parameter
description to list only str and os.PathLike, removing the unsupported file-like
object claim; keep the implementation unchanged unless explicit file-like
handling is added.
In `@dpdata/plugins/amber.py`:
- Around line 19-21: Update the documentation near from_labeled_system to
describe the .mdfrc force trajectory as required, while keeping .mden and .mdout
documented as optional energy and box sources.
- Around line 42-44: The `use_element_symbols` documentation incorrectly
advertises boolean and string-list types. Update the parameter docs at
dpdata/plugins/amber.py lines 42-44 and 92-93 to describe the supported list of
atom indices or Amber-mask string, including that these select atoms whose
element symbols should be used during loading.
In `@dpdata/plugins/deepmd.py`:
- Around line 308-312: Update the docstring for from_multi_systems to describe
its generator behavior rather than listing a list return type. Replace the
Returns section with Yields: str, documenting that each yielded value is a
directory containing type_map.raw and mixed-type data.
In `@dpdata/plugins/qe.py`:
- Around line 24-31: Update the QE reader logic using file_name so os.PathLike
inputs are normalized before appending the ".in", ".pos", ".evp", and ".for"
suffixes, preserving the documented path-like API; apply this consistently
across the CP and PWscf readers.
In `@dpdata/plugins/xyz.py`:
- Around line 26-29: Update the XYZ usage example in the `to` documentation so
it is self-contained: add the necessary import and create a system instance
assigned to `s` before invoking `s.to("xyz", "a.xyz")`.
---
Outside diff comments:
In `@dpdata/plugins/deepmd.py`:
- Around line 492-507: Update the HDF5 group generator docstring near
from_labeled_system/from_system to document both consumers: groups are passed to
from_labeled_system by default, and to from_system only when labeled=False.
Correct the parameter or description text without changing runtime behavior.
---
Nitpick comments:
In `@docs/make_format.py`:
- Around line 522-533: Update the Parameters filtering in the documentation
generation flow to reuse the existing internal_parameters definition instead of
duplicating the hidden-name set. Ensure the filtering includes the current
method’s internal parameters and preserves the union of relevant entries when a
shared to_system implementation serves to_bond_order_system documentation.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 928033dc-65d0-4a00-a358-afeac6adf3bf
📒 Files selected for processing (37)
docs/conf.pydocs/formats.rstdocs/make_format.pydpdata/driver.pydpdata/formats/abacus/relax.pydpdata/formats/abacus/stru.pydpdata/formats/amber/md.pydpdata/formats/gaussian/fchk.pydpdata/formats/lmdb/format.pydpdata/formats/rdkit/sanitize.pydpdata/plugins/3dmol.pydpdata/plugins/abacus.pydpdata/plugins/amber.pydpdata/plugins/ase.pydpdata/plugins/cp2k.pydpdata/plugins/deepmd.pydpdata/plugins/dftbplus.pydpdata/plugins/fhi_aims.pydpdata/plugins/gaussian.pydpdata/plugins/gromacs.pydpdata/plugins/lammps.pydpdata/plugins/list.pydpdata/plugins/n2p2.pydpdata/plugins/openmx.pydpdata/plugins/orca.pydpdata/plugins/psi4.pydpdata/plugins/pwmat.pydpdata/plugins/pymatgen.pydpdata/plugins/qe.pydpdata/plugins/rdkit.pydpdata/plugins/siesta.pydpdata/plugins/vasp.pydpdata/plugins/xyz.pydpdata/system.pydpdata/utils.pypyproject.tomltests/test_format_docs.py
💤 Files with no reviewable changes (2)
- pyproject.toml
- dpdata/formats/rdkit/sanitize.py
| def from_system(self, file_name, **kwargs): | ||
| """Load one ABACUS ``STRU`` file. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| file_name : str or os.PathLike or file-like object | ||
| Input ``STRU`` file. | ||
| **kwargs : dict | ||
| Additional format arguments accepted for API compatibility. | ||
|
|
||
| Returns | ||
| ------- | ||
| dict | ||
| System data, including optional ``move`` and magnetic fields when | ||
| present in the file. | ||
| """ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the unsupported file-like input claim.
from_system passes file_name directly to get_frame_from_stru, which treats it as a filesystem path; file-like objects are not accepted. Change Line 36 to str or os.PathLike, or add explicit file-like handling before documenting it. (raw.githubusercontent.com)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dpdata/plugins/abacus.py` around lines 31 - 46, Update the from_system
docstring’s file_name parameter description to list only str and os.PathLike,
removing the unsupported file-like object claim; keep the implementation
unchanged unless explicit file-like handling is added.
Source: MCP tools
| Labeled loading additionally combines optional ``.mdfrc``, ``.mden``, | ||
| and ``.mdout`` files for forces, energies, and box information. The | ||
| ``parmed`` optional dependency is required. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Mark the force trajectory as required.
from_labeled_system always opens mdfrc_file; only the energy sources are conditional. Describing .mdfrc as optional causes labeled loads without it to fail.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dpdata/plugins/amber.py` around lines 19 - 21, Update the documentation near
from_labeled_system to describe the .mdfrc force trajectory as required, while
keeping .mden and .mdout documented as optional energy and box sources.
| use_element_symbols : list[str] or bool, optional | ||
| Element-symbol information forwarded to the AMBER reader when | ||
| atom names cannot be inferred unambiguously from the topology. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the use_element_symbols contract in both reader docs.
The reader accepts a list of atom indices or an Amber-mask string; bool and list[str] are not supported. Document the actual accepted types and semantics.
dpdata/plugins/amber.py#L42-L44: change the type and describe index-list or mask-string behavior.dpdata/plugins/amber.py#L92-L93: make the same correction for labeled loading.
📍 Affects 1 file
dpdata/plugins/amber.py#L42-L44(this comment)dpdata/plugins/amber.py#L92-L93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dpdata/plugins/amber.py` around lines 42 - 44, The `use_element_symbols`
documentation incorrectly advertises boolean and string-list types. Update the
parameter docs at dpdata/plugins/amber.py lines 42-44 and 92-93 to describe the
supported list of atom indices or Amber-mask string, including that these select
atoms whose element symbols should be used during loading.
| Returns | ||
| ------- | ||
| list[str] | ||
| Directories containing ``type_map.raw`` and mixed-type data. | ||
| """ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document this API as an iterator, not a list.
from_multi_systems contains yield, so callers receive an iterator. The Returns: list[str] section is inaccurate; document Yields: str or an iterator return type instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dpdata/plugins/deepmd.py` around lines 308 - 312, Update the docstring for
from_multi_systems to describe its generator behavior rather than listing a list
return type. Replace the Returns section with Yields: str, documenting that each
yielded value is a directory containing type_map.raw and mixed-type data.
| file_name : str or os.PathLike | ||
| Common prefix of the CP input and trajectory files. | ||
| begin : int, default=0 | ||
| Index of the first frame to load. | ||
| step : int, default=1 | ||
| Load every ``step``-th frame. | ||
| **kwargs : dict | ||
| Additional format arguments accepted for API compatibility. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
python - <<'PY'
from pathlib import Path
try:
Path("run") + ".in"
except TypeError:
print("Path concatenation fails as expected")
PYRepository: deepmodeling/dpdata
Length of output: 194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | grep -E '(^|/)dpdata/plugins/qe\.py$' || true
echo "== outline =="
ast-grep outline dpdata/plugins/qe.py --view expanded || true
echo "== relevant file sections =="
cat -n dpdata/plugins/qe.py | sed -n '1,130p'
echo "== search path ops in qe.py =="
python3 - <<'PY'
from pathlib import Path
p=Path('dpdata/plugins/qe.py')
lines=p.read_text().splitlines()
for i,l in enumerate(lines, 1):
if '+ ' in l or '.fsdecode' in l or 'fspath' in l or os_path_like in l:
print(i,l)
PY
echo "== ruff/format check, no repo mutation =="
if command -v ruff >/dev/null 2>&1; then
ruff check dpdata/plugins/qe.py
else
echo "ruff not available"
fiRepository: deepmodeling/dpdata
Length of output: 5318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate modules =="
git ls-files | grep -E '^dpdata/(formats|formats/qe|formats/qe/scf|formats/qe/traj).*\.py$' || true
echo "== qe traj outline =="
ast-grep outline dpdata/formats/qe/traj.py --view expanded || true
echo "== qe scf outline =="
ast-grep outline dpdata/formats/qe/scf.py --view expanded || true
echo "== file_name concatenations/usages =="
rg -n 'from_system |from_labeled_system |get_frame|file_name|\\.in|\\.pos|\\.evp|\\.for|open\\(|with open' dpdata/formats dpdata/plugins/qe.pyRepository: deepmodeling/dpdata
Length of output: 3912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
paths = [Path("run"), Path("run"), Path("pw.out")]
cases = [
("QE CP from_system", paths[0], ".in", ".pos", ".evp", ".for"),
("QE CP labeled", paths[1], ".in", ".pos", ".evp", ".for"),
("QE PW", paths[2]),
]
for name, prefix, *suffixes in cases:
print(f"\n{name}: prefix={prefix!r}, type={type(prefix).__name__}")
for suffix in suffixes or [""]:
try:
print(" add:", repr(prefix + suffix))
except Exception as exc:
print(" add:", type(exc).__name__, repr(str(exc)))
PYRepository: deepmodeling/dpdata
Length of output: 1008
Normalize path-like file names or narrow the documented type.
qpdata/plugins/qe.py documents file_name as str or os.PathLike, but the QE CP and PWscf readers concatenate it with suffixes like ".in", ".pos", ".evp", and ".for", causing pathlib.Path inputs to fail. Convert path-like inputs to paths before concatenation, or document str only.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dpdata/plugins/qe.py` around lines 24 - 31, Update the QE reader logic using
file_name so os.PathLike inputs are normalized before appending the ".in",
".pos", ".evp", and ".for" suffixes, preserving the documented path-like API;
apply this consistently across the CP and PWscf readers.
| Examples | ||
| -------- | ||
| >>> s.to("xyz", "a.xyz") | ||
| """ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the XYZ example self-contained.
Line 28 uses s without defining it. Add an import and construct a system before calling to().
Proposed fix
+ >>> import dpdata
+ >>> s = dpdata.System("POSCAR", fmt="vasp/poscar")
>>> s.to("xyz", "a.xyz")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Examples | |
| -------- | |
| >>> s.to("xyz", "a.xyz") | |
| """ | |
| Examples | |
| -------- | |
| >>> import dpdata | |
| >>> s = dpdata.System("POSCAR", fmt="vasp/poscar") | |
| >>> s.to("xyz", "a.xyz") | |
| """ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dpdata/plugins/xyz.py` around lines 26 - 29, Update the XYZ usage example in
the `to` documentation so it is self-contained: add the necessary import and
create a system instance assigned to `s` before invoking `s.to("xyz", "a.xyz")`.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
Summary
**kwargsoptions; make generation independent of the caller working directory.Validation
ruff format dpdata docs/conf.py docs/make_format.py tests/test_format_docs.pyruff check dpdata/ docs/conf.py docs/make_format.py tests/test_format_docs.pypython -m unittest discover— 2238 tests passed, 28 skippedpython -m unittest discover -s tests -p 'test_format_docs.py'dpdata --helpdpdata --versionDPDATA_DOCS_SKIP_JUPYTERLITE=1 sphinx-build -a -b html . _build/html— succeeded; 1650 existing API/autosummary warning lines remain, with 0 warnings fromdocs/formats.rstordocs/formats/*.rstNotes
The default JupyterLite build still requires micromamba.
DPDATA_DOCS_SKIP_JUPYTERLITE=1skips only that optional extension so contributors can validate the regular Sphinx pages. Existing repository-wide API/autosummary warnings remain outside this documentation-focused change.Coding agent: Codex
Codex version: codex-cli 0.144.6
Model: gpt-5.6-sol
Reasoning effort: xhigh
Summary by CodeRabbit
Documentation
Bug Fixes
Tests
Changes