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
Merging this PR will improve performance by 29.16%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | test_cli |
390.5 ms | 289.2 ms | +35.04% |
| ⚡ | WallTime | test_import |
11.5 ms | 9.3 ms | +23.54% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing njzjz:docs/expand-format-reference (e6f267d) with master (c77e038)
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.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR expands format documentation, restructures generated format references, updates 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 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 |
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
|
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 |
…to all format class docstrings Each format class now includes a brief introduction of the computational software and a link to its official website in the class docstring, following the pattern established by OpenMX and DFTB+ formats. Coding-Agent: opencode opencode-Version: 1.17.18 Model: ustc/deepseek-v4-pro Reasoning-Effort: max
Insert blank line between summary line and description per ruff D205 rule. Coding-Agent: opencode opencode-Version: 1.17.18 Model: ustc/deepseek-v4-pro Reasoning-Effort: max
Address review feedback on the expanded format reference: - `open_file`: document `*args` as a tuple and finish the truncated `ValueError` description. - `abacus/stru`: drop the file-like input claim; `get_frame_from_stru` only accepts a filesystem path. - `amber/md`: mark `.mdfrc` as required for labeled loading and describe `use_element_symbols` as an atom-index list or an AMBER mask string. - `qe/cp/traj`, `qe/pw/scf`: `file_name` is concatenated with suffixes and type-checked against `str`, so `os.PathLike` is not accepted; also document the `[input, output]` pair accepted by `qe/pw/scf`. - `xyz`: make the class example self-contained. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Address the two review comments CodeRabbit could not post inline: - `DeePMDHDF5Format.from_multi_systems`: `MultiSystems.from_fmt_obj` sends each yielded group to `from_labeled_system` by default and to `from_system` only when `labeled=False`; say so instead of naming only the unlabeled path. - `docs/make_format.py`: derive the hidden writer-parameter set from `internal_parameters` instead of repeating the names, so the two cannot drift. The union is kept rather than narrowing per method, since one `to_system` implementation can back several writer pages; the resulting set is identical to the previous literal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Also addressed the two CodeRabbit comments that could not be posted inline, in e6f267d:
|
wanghan-iapcm
left a comment
There was a problem hiding this comment.
Nice cleanup -- the cwd-independent generation and the per-alias signature fix are real improvements. A few accuracy issues in the generated output, found by regenerating the pages against master and diffing.
| buff.append(""" :noindex:""") | ||
| buff.append("") | ||
| if docstring is None or method not in format.__dict__: | ||
| if docstring is None: |
There was a problem hiding this comment.
Dropping the method not in format.__dict__ guard from #516 means inherited base-class implementations now surface their plugin-author docstrings on user-facing pages.
check_supported() advertises from_multi_systems/to_multi_systems whenever MultiMode != NotImplemented, so getattr resolves to dpdata/format.py's base methods, whose text reads "Implement MultiSystems.from that converts from this format to MultiSystems. By default, this method follows MultiMode..." -- plus an internal **kwargs : keyword arguments that will be passed from the method row.
Regenerating on master vs this branch: master emits Convert this format to :class:`MultiSystems`. in all these places; this branch leaks the internal text in 5 (DeePMDCompFormat.rst 285/333, DeePMDRawFormat.rst 266/314, DeePMDMixedFormat.rst 226). Same applies to the to_ branch at line 522.
Could you restore the __dict__ check, keeping the to_labeled_system fallback that get_method_obj now handles?
| return arguments, has_location | ||
|
|
||
|
|
||
| def append_quick_examples( |
There was a problem hiding this comment.
These are advertised as copyable, but for write-only formats the write example names a variable the block never defines, so pasting raises NameError.
11 of 51 generated pages are affected: ListFormat, Py3DMolFormat, VASPStringFormat, PSI4InputFormat, SQMINFormat, PyMatgenCSEFormat, plus DeePMDMixedFormat and DeePMDHDF5MixedFormat (these two define only systems from MultiSystems.from_file, then call system.to(...)), and undefined atoms/molecule/structure on the ASE and pymatgen pages.
This is the same defect you fixed for the hand-written xyz example in e9a0402 -- worth fixing in the generator too, either by emitting a construction stub or marking these blocks as non-runnable.
| "AmberMDFormat": '"trajectory_prefix"', | ||
| "ASEStructureFormat": "atoms", | ||
| "CP2KAIMDOutputFormat": '"calculation_directory"', | ||
| "DFTBplusFormat": '("geometry.gen", "detailed.out")', |
There was a problem hiding this comment.
This points users at the wrong file. read_dftb_plus scans for line.startswith("Geometry") and parses the brace block, i.e. it needs the DFTB+ HSD input, not a standalone GenFormat file -- the repo's own fixture tests/dftbplus/dftb_pin.hsd starts with Geometry = GenFormat {. An actual .gen file (which begins with the atom-count line) raises ValueError("GenFormat Geometry block not found in DFTB+ input").
Suggest ("dftb_in.hsd", "detailed.out").
| `PWmat <https://www.pwmat.com/>`_ is a plane-wave DFT software using GPU | ||
| acceleration. | ||
|
|
||
| The format stores a cell and one atomic configuration. Reading and writing |
There was a problem hiding this comment.
"Reading and writing normalize the cell to dpdata's lower-triangular convention" is only true for reading. from_system carries @Format.post("rot_lower_triangular"), but to_system has no decorator and dpdata/formats/pwmat/atomconfig.py::from_system_data writes system["cells"][f_idx] verbatim.
Suggest dropping "and writing".
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