Conversation
hoyer-a
left a comment
There was a problem hiding this comment.
Generally looking good!
In the other packages, the module page titles are like their import paths (e.g., pyrato.dsp). That's also what appears in the left sidebar.
Maybe change it for consistency, also for analytic.
suggestions to documentation hierarchy Co-authored-by: Anton Hoyer <156099087+hoyer-a@users.noreply.github.com>
|
The structure is adjusted for analytic submodule. All warnings from the compilation are taken care of. Only two from the example code reside but I will take care of those another day. |
Are they coming from code that you touched? If inot, it should be fixed in a different pull. We try to be strict about that to make the pulls smaller, and easier to review. |
No they are in the example code in the docstrings. So they are from an earlier iteration. |
ahms5
left a comment
There was a problem hiding this comment.
thanks, looks fine, in my opinion we are almost there. I dont see any other warinings anymore. you can simply check locally:
make html SPHINXOPTS="-W"
docs/modules/analytic/impedance.rst
Outdated
| @@ -0,0 +1,7 @@ | |||
| pyrato.analytic.impedance | |||
docs/modules/analytic/analytic.rst
Outdated
| @@ -0,0 +1,7 @@ | |||
| pyrato.analytic.analytic | |||
There was a problem hiding this comment.
delete,
we dont need this level of detail, this subdevision of the analytic model is just internally. From extern you still call all funtions from analytic. e.g. pyrato.analytic.rectangular_room_rigid_walls which is in analytic.py or pyrato.analytic.rectangular_room_impedance from impedance.py. The documentaiton should habe the same style
Co-authored-by: Anne Heimes <64446926+ahms5@users.noreply.github.com>
Thanks! Two warnings are being put out upon compilation but they stem from the internal python examples in the Docstrings. In that case this would be a new PR I think ... |
mberz
left a comment
There was a problem hiding this comment.
Looking good. I currently cannot trigger the circleci test manually, but they should start if there's another commit on this PR.
The mentioned issues can be taken care of in a separate PR
Which issue(s) are closed by this pull request?
API references adjusted for v_1.0.0
Changes proposed in this pull request: