Conversation
Feeding summarizer changes into the documentation branch
There was a problem hiding this comment.
Pull request overview
This PR improves the documentation for the MMIF summarizer and CLI scripts, addressing issue #348 about CLI script documentation organization and formatting.
Changes:
- Moved high-level summarizer documentation from module docstrings to dedicated documentation files
- Added proper Sphinx/reStructuredText formatting to docstrings (using
:var:,:param:,:meta private:directives) - Created new documentation pages for the summarizer with usage examples
- Updated module index to include the summarizer package documentation
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| mmif/utils/summarizer/init.py | Moved module-level documentation to package docstring, removed outdated code comments |
| mmif/utils/summarizer/utils.py | Updated module docstring formatting |
| mmif/utils/summarizer/summary.py | Converted docstring comments to proper Sphinx format with :var: directives |
| mmif/utils/summarizer/graph.py | Added comprehensive documentation with proper Sphinx formatting, type hints, and :meta private: directives |
| mmif/utils/cli/summarize.py | Added docstrings for functions with type hints |
| mmif/utils/cli/describe.py | Minor formatting improvements to docstring |
| documentation/summarizer.rst | New documentation file for the summarizer with usage examples |
| documentation/modules.rst | Added summarizer package to module index |
| documentation/index.rst | Added summarizer to documentation table of contents |
| documentation/cli.rst | Minor wording improvement |
| documentation/autodoc/mmif.utils.summarizer.rst | New autodoc configuration for summarizer package |
| documentation/autodoc/mmif.utils.cli.rst | Added summarize module to CLI autodoc |
| documentation-notes.md | Added note about source links in documentation |
| .gitignore | Changed file extension for whatsnew from .rst to .md |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - The time unit is assumed to be milliseconds. | ||
|
|
||
|
|
||
| The summarizer is accessable via the ``mmif`` command line script. To run the |
There was a problem hiding this comment.
Spelling error: "accessable" should be "accessible".
| The summarizer is accessable via the ``mmif`` command line script. To run the | |
| The summarizer is accessible via the ``mmif`` command line script. To run the |
|
|
||
| .. code-block:: bash | ||
|
|
||
| mmif summarize -i INFILE -o OUTFILE |
There was a problem hiding this comment.
Inconsistent indentation: The code block indentation uses a tab character here, but other code blocks in the documentation (cli.rst, introduction.rst, plugins.rst) use spaces. For consistency, this should use spaces instead of a tab.
|
closing without merge, I found the 348 branch already previously PREMATURELY merged into the develop branch WITHOUT a proper PR, so I'm reverting it to get a sole working branch first. |
|
Ah, I see this is probably my old habit of local merging popping up. As for the premature bit, this begs the question of what premature is in the context of a merge into develop. I think the more useful approach is to get to a good handle on when exactly we should request reviews on PRs. Another thought is that before merging into develop we could do a merge from develop, but I am not that enamored with having those back and forth merges all over the place. |
2 participants
addresses #348 and other various minor documentation issues