feat: provide mechanism to link between report items (snakemake.report_href, see docs)#3224
feat: provide mechanism to link between report items (snakemake.report_href, see docs)#3224johanneskoester merged 6 commits intomainfrom
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant enhancements to the Snakemake reporting framework. Key modifications include the addition of new methods and directives for generating reports, improved handling of file paths, and the introduction of dynamic linking capabilities. Documentation has been updated to reflect these changes, including examples and usage instructions. New HTML files and a test case have been added to validate the functionality of the report generation features, ensuring comprehensive coverage of the new capabilities. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
tests/test_report_href/test_script.py (1)
3-3: Suppress static analysis warnings forsnakemakebeing undefinedStatic analysis tools may flag
snakemakeas undefined because it's injected by Snakemake at runtime. To improve code clarity and suppress these warnings, consider adding a type hint forsnakemake.Add the following at the top of your script:
from snakemake.script import Snakemake snakemake: Snakemake🧰 Tools
🪛 Ruff (0.8.0)
3-3: Undefined name
snakemake(F821)
snakemake/report/html_reporter/template/components/content.js (1)
68-68: Ensure consistent re-rendering without unnecessary unmountingUsing
renderTriggeras a key for theiframeworks with the updated incremental counter, but it's generally better to control re-renders through state or props changes. If the content changes are based oncontentPath, consider using it directly as the key.Modify the
iframeelement to usecontentPathas the key:return e( "iframe", - { src: this.props.app.state.contentPath, className: "w-full h-screen", key: `${this.props.app.state.renderTrigger}` } + { src: this.props.app.state.contentPath, className: "w-full h-screen", key: this.props.app.state.contentPath } );tests/tests.py (2)
258-260: Add docstring to explain test purpose.Consider adding a docstring to explain what aspect of the report href functionality is being tested. This helps other developers understand the test's purpose and requirements.
def test_report_href(): + """Test the functionality of generating reports with hyperlinks using snakemake.report_href().""" run(dpath("test_report_href"))
260-261: Remove extra empty line.There are two consecutive empty lines where one would be sufficient according to PEP 8.
def test_report_href(): run(dpath("test_report_href")) - def test_report_zip():
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
docs/snakefiles/reporting.rst(4 hunks)snakemake/common/__init__.py(2 hunks)snakemake/report/__init__.py(7 hunks)snakemake/report/html_reporter/template/components/app.js(2 hunks)snakemake/report/html_reporter/template/components/content.js(1 hunks)snakemake/script/__init__.py(4 hunks)tests/test_report_href/Snakefile(1 hunks)tests/test_report_href/expected-results/test2.html(1 hunks)tests/test_report_href/subdir/subdir/test3.html(1 hunks)tests/test_report_href/test.html(1 hunks)tests/test_report_href/test_script.py(1 hunks)tests/tests.py(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- tests/test_report_href/subdir/subdir/test3.html
- tests/test_report_href/expected-results/test2.html
- tests/test_report_href/test.html
🧰 Additional context used
📓 Path-based instructions (5)
tests/test_report_href/test_script.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
tests/tests.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
snakemake/common/__init__.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
snakemake/script/__init__.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
snakemake/report/__init__.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
🪛 Ruff (0.8.0)
tests/test_report_href/test_script.py
3-3: Undefined name snakemake
(F821)
11-11: Undefined name snakemake
(F821)
12-12: Undefined name snakemake
(F821)
snakemake/script/__init__.py
90-93: Use ternary operator anchor = f"#{urllib.parse.quote(self._anchor)}" if self._anchor else "" instead of if-else-block
Replace if-else-block with anchor = f"#{urllib.parse.quote(self._anchor)}" if self._anchor else ""
(SIM108)
🔇 Additional comments (8)
tests/test_report_href/Snakefile (1)
1-13: Rule a is correctly defined
The rule a is properly configured with inputs, outputs, and script. The use of report functions and the pattern matching in inputs are appropriate.
snakemake/report/__init__.py (3)
Line range hint 360-443: LGTM: FileRecord class changes look good.
The addition of parent_path and the modified filename property correctly handle relative paths for files within directories.
Line range hint 545-565: LGTM: register_file function signature update.
The addition of the parent_path parameter and its handling in the FileRecord creation is implemented correctly.
🧰 Tools
🪛 Ruff (0.8.0)
551-551: Function definition does not bind loop variable job
(B023)
613-632: Improved error handling for pattern matching.
Good addition of the found_something flag to warn users when no files match the given patterns.
docs/snakefiles/reporting.rst (2)
Line range hint 134-194: Well-documented linking functionality.
The documentation clearly explains the new linking capabilities and provides good examples.
196-238: Comprehensive examples for advanced linking features.
Good coverage of child_path, url_args, and anchor functionality with clear examples.
snakemake/common/__init__.py (2)
20-20: LGTM!
Clean addition of the Union type import, required for type hinting in the new get_report_id function.
56-60: LGTM! Clean and focused implementation.
The function:
- Properly handles both string and Path objects
- Uses a cryptographically secure hash function (SHA256)
- Has appropriate type hints
- Follows the single responsibility principle
Let's verify the usage of this function in the reporting framework:
✅ Verification successful
Function is properly integrated into the reporting framework
The verification shows that get_report_id is correctly used in the reporting framework:
- Imported and used in
snakemake/report/__init__.pyto generate unique IDs for reports - Also integrated in
snakemake/script/__init__.pyfor script identification - Both usages align with the function's purpose of generating unique identifiers from paths
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of get_report_id in the reporting framework
# Expected: Function should be used in report-related files for generating IDs
# Search for imports of get_report_id
rg "from snakemake.common import .*get_report_id" -A 2
# Search for direct usage of get_report_id
rg "get_report_id\(" -A 2
Length of output: 1086
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
snakemake/script/__init__.py (1)
52-71: Consider adding input validation.The initialization looks good, but consider adding validation for:
pathto ensure it's a valid report pathurl_argsto ensure values are strings or can be converted to stringsdef __init__( self, path: Union[str, Path], parent: Optional[ReportHrefType] = None, url_args: Optional[Dict[str, str]] = None, anchor: Optional[str] = None, ): + if not isinstance(path, (str, Path)): + raise TypeError(f"path must be str or Path, got {type(path)}") self._parent = parent if parent is None: self._id = get_report_id(path) else: self._id = parent._id # ensure that path is a url compatible string self._path = path if isinstance(path, str) else str(path.as_posix()) self._url_args = ( - {key: value for key, value in url_args.items()} if url_args else {} + {key: str(value) for key, value in url_args.items()} if url_args else {} ) self._anchor = anchor
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
snakemake/script/__init__.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
snakemake/script/__init__.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
🪛 Ruff (0.8.0)
snakemake/script/__init__.py
93-96: Use ternary operator anchor = f"#{urllib.parse.quote(self._anchor)}" if self._anchor else "" instead of if-else-block
Replace if-else-block with anchor = f"#{urllib.parse.quote(self._anchor)}" if self._anchor else ""
(SIM108)
🔇 Additional comments (5)
snakemake/script/__init__.py (5)
19-22: LGTM: Import changes are appropriate.
The added imports are necessary for the new functionality and follow Python's import style guide.
48-49: LGTM: Good compatibility solution.
The use of TypeVar for Python <3.11 compatibility is a good solution, addressing the previous review comment about Self type usage.
72-81: LGTM: Helper methods are well-designed.
The methods for creating child paths and modifying URL arguments/anchors are well-designed and maintain immutability by returning new instances.
129-137: LGTM: Well-documented method.
The report_href method is well-documented and follows the single responsibility principle. The docstring clearly explains its purpose and usage.
83-97:
Avoid double URL encoding.
The values in self._url_args are already URL-encoded during initialization. Applying urllib.parse.quote again in the fmt_arg function results in double encoding.
Apply this diff to fix the issue:
def fmt_arg(key, value):
- return f"{key}={urllib.parse.quote(str(value))}"
+ return f"{key}={str(value)}"Also, the same issue exists with anchor encoding:
if self._anchor:
- anchor = f"#{urllib.parse.quote(self._anchor)}"
+ anchor = f"#{self._anchor}"
else:
anchor = ""🧰 Tools
🪛 Ruff (0.8.0)
93-96: Use ternary operator anchor = f"#{urllib.parse.quote(self._anchor)}" if self._anchor else "" instead of if-else-block
Replace if-else-block with anchor = f"#{urllib.parse.quote(self._anchor)}" if self._anchor else ""
(SIM108)
🤖 I have created a release *beep* *boop* --- ## [8.29.0](v8.28.0...v8.29.0) (2025-02-27) ### Features * provide mechanism to link between report items (snakemake.report_href, see docs) ([#3224](#3224)) ([a024e60](a024e60)) * toggles for binary labels in report ([#3296](#3296)) ([7690998](7690998)) ### Bug Fixes * add missing [@conda](https://github.com/conda) annotations ([#3298](#3298)) ([0b541cc](0b541cc)) * properly handle remote storage when waiting for pipes ([#3317](#3317)) ([9f7e092](9f7e092)) * provide proper error messages if output file preparation (pipe, mkdir) fails ([#3309](#3309)) ([77ef176](77ef176)) * record file extension (e.g. .tsv) in between workflow cache records in order to avoid that files of the wrong format are erroneously returned by the cache. This will lead to some cache misses. But avoiding errors induced by the previous behavior of ignoring file extensions when seeking for cache records is more important. ([#3314](#3314)) ([4912f00](4912f00)) * sort results lexicographically by labels in their order of appearance ([#3293](#3293)) ([a19a0ac](a19a0ac)) ### Documentation * update best practices ([7270eb3](7270eb3)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…t_href, see docs) (snakemake#3224) <!--Add a description of your PR here--> ### QC <!-- Make sure that you can tick the boxes below. --> * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Enhanced reporting capabilities with new directives and methods for generating comprehensive HTML reports. - Introduced dynamic linking in reports and the ability to include file labels and categories. - Added new HTML report generation functionality with example workflows. - New functionality to manage report hyperlinks, improving navigation within reports. - **Bug Fixes** - Improved error handling and user feedback during report generation. - **Tests** - Added new tests for report generation functionality to ensure reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🤖 I have created a release *beep* *boop* --- ## [8.29.0](snakemake/snakemake@v8.28.0...v8.29.0) (2025-02-27) ### Features * provide mechanism to link between report items (snakemake.report_href, see docs) ([snakemake#3224](snakemake#3224)) ([a024e60](snakemake@a024e60)) * toggles for binary labels in report ([snakemake#3296](snakemake#3296)) ([7690998](snakemake@7690998)) ### Bug Fixes * add missing [@conda](https://github.com/conda) annotations ([snakemake#3298](snakemake#3298)) ([0b541cc](snakemake@0b541cc)) * properly handle remote storage when waiting for pipes ([snakemake#3317](snakemake#3317)) ([9f7e092](snakemake@9f7e092)) * provide proper error messages if output file preparation (pipe, mkdir) fails ([snakemake#3309](snakemake#3309)) ([77ef176](snakemake@77ef176)) * record file extension (e.g. .tsv) in between workflow cache records in order to avoid that files of the wrong format are erroneously returned by the cache. This will lead to some cache misses. But avoiding errors induced by the previous behavior of ignoring file extensions when seeking for cache records is more important. ([snakemake#3314](snakemake#3314)) ([4912f00](snakemake@4912f00)) * sort results lexicographically by labels in their order of appearance ([snakemake#3293](snakemake#3293)) ([a19a0ac](snakemake@a19a0ac)) ### Documentation * update best practices ([7270eb3](snakemake@7270eb3)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
QC
docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests