4176 docstrings still in the rst format#4213
Conversation
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
…e docstring for parameters Co-authored-by: Copilot <copilot@github.com>
…meters and return values Co-authored-by: Copilot <copilot@github.com>
…r parameters Co-authored-by: Copilot <copilot@github.com>
…or parameters Co-authored-by: Copilot <copilot@github.com>
…tyle for parameters Co-authored-by: Copilot <copilot@github.com>
e944e7e to
b4b7515
Compare
There was a problem hiding this comment.
Pull request overview
This PR continues the migration away from reStructuredText-style docstrings toward Numpy-style docstrings across several PROCESS modules, and adds/updates some type annotations to improve readability and tooling support.
Changes:
- Converted multiple docstrings from
:param/:typeRST format to Numpy docstring sections. - Added/updated type annotations on a few constructors/initializers.
- Introduced new imports to support the added type annotations.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| process/models/physics/plasma_current.py | Updates a function docstring to Numpy style for the Peng current coefficient helper. |
| process/models/physics/impurity_radiation.py | Adds a PlasmaProfile type annotation and converts __init__ docstring to Numpy style. |
| process/models/ife.py | Converts IFE.__init__ docstring to Numpy style. |
| process/models/blankets/blanket_library.py | Converts several blanket segment-length docstrings to Numpy style. |
| process/core/solver/evaluators.py | Adds typing imports/annotations and converts Evaluators.__init__ docstring to Numpy style. |
| process/core/scan.py | Adds typing imports/annotations and converts Scan.__init__ docstring to Numpy style. |
| process/core/log.py | Adds a type annotation to ProcessLogHandler.__init__ and converts docstring to Numpy style. |
| process/core/io/in_dat/base.py | Adds a str type annotation for StructuredInputData.__init__ and converts docstring to Numpy style. |
| process/core/caller.py | Converts Caller.__init__ docstring to Numpy style. |
Comments suppressed due to low confidence (2)
process/core/scan.py:33
from process.main import Modelsintroduces an import cycle:process/main.pyimportsprocess.core.scan.Scan, andscan.pynow importsModelsback fromprocess.main, which will break module import/CLI startup. Use postponed annotations +TYPE_CHECKING(or quote the annotation) soModelsis only imported for type checking, not at runtime.
process/core/solver/evaluators.py:13from process.main import Modelshere is a runtime dependency that can create an import cycle (solver/scan/main import chain) and is only needed for type hints. Preferfrom __future__ import annotationsand aTYPE_CHECKING-guarded import (or quotingModels) to avoid importingprocess.mainat module import time.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| :rtype: float | ||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
The Returns section is using : as the return type/name placeholder. In Numpydoc this should be a concrete type (e.g. float) so that Sphinx/numpydoc can render it correctly.
| : | |
| float |
| n_blkt_outboard_modules_poloidal : | ||
| Number of outboard blanket modules in poloidal direction | ||
| dr_fw_plasma_gap_inboard : | ||
| Radial gap between inboard first wall and plasma (m) | ||
| rminor : | ||
| Minor radius of the plasma (m) | ||
| dr_fw_plasma_gap_outboard : | ||
| Radial gap between outboard first wall and plasma (m) | ||
| dz_blkt_half : | ||
| Half-height of the blanket module (m) | ||
| n_divertors : | ||
| Number of divertors (1 for single null, 2 for double null) | ||
| f_ster_div_single : | ||
| Fractional poloidal length of the divertor in single null configuration | ||
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
These Parameters entries omit the types after the colon (contrast with other functions in this module that use name : float/int). Adding the types will keep docstrings consistent and more reliably parsed by Numpydoc/Sphinx.
| n_blkt_outboard_modules_poloidal : | |
| Number of outboard blanket modules in poloidal direction | |
| dr_fw_plasma_gap_inboard : | |
| Radial gap between inboard first wall and plasma (m) | |
| rminor : | |
| Minor radius of the plasma (m) | |
| dr_fw_plasma_gap_outboard : | |
| Radial gap between outboard first wall and plasma (m) | |
| dz_blkt_half : | |
| Half-height of the blanket module (m) | |
| n_divertors : | |
| Number of divertors (1 for single null, 2 for double null) | |
| f_ster_div_single : | |
| Fractional poloidal length of the divertor in single null configuration | |
| Returns | |
| ------- | |
| : | |
| n_blkt_outboard_modules_poloidal : int | |
| Number of outboard blanket modules in poloidal direction | |
| dr_fw_plasma_gap_inboard : float | |
| Radial gap between inboard first wall and plasma (m) | |
| rminor : float | |
| Minor radius of the plasma (m) | |
| dr_fw_plasma_gap_outboard : float | |
| Radial gap between outboard first wall and plasma (m) | |
| dz_blkt_half : float | |
| Half-height of the blanket module (m) | |
| n_divertors : int | |
| Number of divertors (1 for single null, 2 for double null) | |
| f_ster_div_single : float | |
| Fractional poloidal length of the divertor in single null configuration | |
| Returns | |
| ------- | |
| float |
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
The Returns section uses : as a placeholder rather than an explicit return type. Please change this to the appropriate type (likely float) for correct Numpydoc rendering.
| : | |
| float |
|
|
||
| Parameters | ||
| ---------- | ||
| plasma_profile : |
There was a problem hiding this comment.
This docstring was converted to Numpy style but the parameter line omits the type after the colon. Consider using plasma_profile : PlasmaProfile (or a broader protocol/interface type) to match Numpydoc expectations and improve rendered docs.
| plasma_profile : | |
| plasma_profile : PlasmaProfile |
| rmajor : | ||
| Major radius of the plasma (m) | ||
| rminor : | ||
| Minor radius of the plasma (m) | ||
| triang : | ||
| Triangularity of the plasma | ||
| dr_fw_plasma_gap_inboard : | ||
| Radial gap between inboard first wall and plasma (m) | ||
| dz_blkt_half : | ||
| Half-height of the blanket module (m) | ||
| n_blkt_inboard_modules_poloidal : | ||
| Number of inboard blanket modules in poloidal direction | ||
| n_divertors : | ||
| Number of divertors (1 for single null, 2 for double null) | ||
| f_ster_div_single : |
There was a problem hiding this comment.
These Parameters entries omit types after the colon, which is inconsistent with other Numpy-style docstrings in this module (e.g. ... : float). Please add the parameter types to match the established format.
| rmajor : | |
| Major radius of the plasma (m) | |
| rminor : | |
| Minor radius of the plasma (m) | |
| triang : | |
| Triangularity of the plasma | |
| dr_fw_plasma_gap_inboard : | |
| Radial gap between inboard first wall and plasma (m) | |
| dz_blkt_half : | |
| Half-height of the blanket module (m) | |
| n_blkt_inboard_modules_poloidal : | |
| Number of inboard blanket modules in poloidal direction | |
| n_divertors : | |
| Number of divertors (1 for single null, 2 for double null) | |
| f_ster_div_single : | |
| rmajor : float | |
| Major radius of the plasma (m) | |
| rminor : float | |
| Minor radius of the plasma (m) | |
| triang : float | |
| Triangularity of the plasma | |
| dr_fw_plasma_gap_inboard : float | |
| Radial gap between inboard first wall and plasma (m) | |
| dz_blkt_half : float | |
| Half-height of the blanket module (m) | |
| n_blkt_inboard_modules_poloidal : int | |
| Number of inboard blanket modules in poloidal direction | |
| n_divertors : int | |
| Number of divertors (1 for single null, 2 for double null) | |
| f_ster_div_single : float |
| dz_blkt_half : | ||
| Half-height of the blanket module (m) | ||
| n_blkt_inboard_modules_poloidal : | ||
| Number of inboard blanket modules in poloidal direction | ||
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
In this file, other Numpy-style docstrings use param : type (e.g. z_plasma_xpoint_lower : float). These new parameter entries omit the type, which is inconsistent and can reduce doc rendering quality. Consider adding the types after the colon.
| dz_blkt_half : | |
| Half-height of the blanket module (m) | |
| n_blkt_inboard_modules_poloidal : | |
| Number of inboard blanket modules in poloidal direction | |
| Returns | |
| ------- | |
| : | |
| dz_blkt_half : float | |
| Half-height of the blanket module (m) | |
| n_blkt_inboard_modules_poloidal : int | |
| Number of inboard blanket modules in poloidal direction | |
| Returns | |
| ------- | |
| float |
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
The Returns section uses : as a placeholder rather than a return type. Numpydoc expects a type here (e.g. float) so the generated documentation is correct.
| : | |
| float |
| Parameters | ||
| ---------- | ||
| models : | ||
| physics and engineering model objects | ||
| data : |
There was a problem hiding this comment.
This conversion to Numpy-style docstring removed the parameter types (even though other docstrings in this file include them, e.g. previous : float | np.ndarray). Consider using models : Models and data : DataStructure for consistency and better doc rendering.
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
The Returns section uses : as a placeholder instead of a return type. Please use an explicit type (likely float) so Numpydoc renders the return value correctly.
| : | |
| float |
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
The Returns section uses : as a placeholder instead of a type. For Numpydoc-style docstrings, provide the return type (likely float) so documentation builds cleanly.
| : | |
| float |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4213 +/- ##
==========================================
+ Coverage 52.10% 52.15% +0.04%
==========================================
Files 148 148
Lines 30389 30435 +46
==========================================
+ Hits 15835 15874 +39
- Misses 14554 14561 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
| : | |
| float |
There was a problem hiding this comment.
@jwscook Is saying to not put types in the docstring
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
| : | |
| float |
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
| : | |
| float |
|
|
||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
| : | |
| float |
| :rtype: float | ||
| Returns | ||
| ------- | ||
| : |
There was a problem hiding this comment.
| : | |
| float |
Co-authored-by: Copilot <copilot@github.com>
Description
Checklist
I confirm that I have completed the following checks: