RO-crate content appear in the nf-core lint warning if there is a TODO #3493

ningyuxin1999 · 2025-03-17T20:51:51Z

Changes:

The RO-crate-metadata.json is ignored in linting
Added a test script nf_core/pipelines/lint/rocrate_readme_sync.py for synchronizing the content of ROcrate metadata and the repo README.

The warning message mentioned here disappears when you create a new pipeline without deleting the TODO commands. But I still wonder where I should add the --fix flag so that the use can decide whether they want to synchronize the metadata with the readme file.

PR checklist

This comment contains a description of changes (with reason)
CHANGELOG.md is updated
If you've fixed a bug or added code that should be tested, add tests!
Documentation in docs is updated

…o dev-sync

mirpedrol

leaving a couple of comments, otherwise is looking good!

nf_core/pipelines/lint/pipeline_todos.py

nf_core/pipelines/lint/rocrate_readme_sync.py

Co-authored-by: Júlia Mir Pedrol <[email protected]>

ningyuxin1999 · 2025-04-15T12:51:51Z

@mirpedrol Hey Julia, could you maybe please quickly have a look at the changes again and this would be ready to merge? And I'm also not sure why the pending checks are not running ...

^^Thank you very much in advance!

mashehu · 2025-04-15T13:43:04Z

tests/pipelines/lint/test_rocrate_readme_sync.py

+
+        json_path = Path(self.lint_obj.wf_path, "ro-crate-metadata.json")
+        with open(json_path, "w") as f:
+            f.write("{}")


I would parse it as as JSON and just change the description with some random text, to avoid off targets.

mashehu

~~There is some problem with the self-hosted runners atm, so that they don't pick up any jobs. fix is on the way~~
Fixed

codecov · 2025-04-15T15:32:40Z

Codecov Report

Attention: Patch coverage is 69.38776% with 15 lines in your changes missing coverage. Please review.

Project coverage is 76.53%. Comparing base (619df2e) to head (5d8045a).

Files with missing lines	Patch %	Lines
nf_core/pipelines/lint/rocrate_readme_sync.py	68.75%	15 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mashehu · 2025-04-15T15:40:41Z

can we also add a test for the fix functionality?

mirpedrol

some last suggestions 🙂

mirpedrol · 2025-04-15T15:30:58Z

.github/actions/create-lint-wf/action.yml

      working-directory: create-lint-wf

    # Run nf-core pipelines linting
    - name: nf-core pipelines lint
      shell: bash
-      run: nf-core --verbose --log-file log.txt --hide-progress pipelines lint --dir nf-core-testpipeline --fail-ignored --fail-warned
+      run: nf-core --verbose --log-file log.txt --hide-progress pipelines lint --fix rocrate_readme_sync --dir nf-core-testpipeline --fail-ignored --fail-warned


Was this failing with adding the --fix argument? 🤔 I think we shouldn't add it here, since we want the fresh template to be a valid pipeline

I changed the script so the rocreate_readme_sync function would directly copy the description from the README.md if there's no description key found, therefore there won't be an ignored case added in such situation.

mirpedrol · 2025-04-15T15:31:21Z

.github/actions/create-lint-wf/action.yml

@@ -83,7 +83,7 @@ runs:

    - name: nf-core pipelines lint in release mode
      shell: bash
-      run: nf-core --log-file log.txt --hide-progress pipelines lint --dir nf-core-testpipeline --fail-ignored --fail-warned --release
+      run: nf-core --log-file log.txt --hide-progress pipelines lint --dir nf-core-testpipeline --fix rocrate_readme_sync --fail-ignored --fail-warned --release


same as I mentioned before

mirpedrol · 2025-04-15T15:31:44Z

log.txt

can be removed

nf_core/pipelines/lint/rocrate_readme_sync.py

Co-authored-by: Júlia Mir Pedrol <[email protected]>

ningyuxin1999 · 2025-04-16T12:54:47Z

can we also add a test for the fix functionality?

Yes, I added a new test into the test script.

mirpedrol

This is what I would do with the description, it adds it when we use the flag --fix, and the test is ignored if we don't have a description. The CI test should pass, but showt if it doesn't and I will have another look.

mirpedrol · 2025-04-16T15:42:41Z

nf_core/pipelines/lint/rocrate_readme_sync.py

+        if "description" not in graph[0]:
+            graph[0]["description"] = readme_content
+            passed.append(
+                "No description found in `ro-crate-metadata.json`, add the same description from `README.md` to the RO-Crate metadata."
+            )


Suggested change

if "description" not in graph[0]:

graph[0]["description"] = readme_content

passed.append(

"No description found in `ro-crate-metadata.json`, add the same description from `README.md` to the RO-Crate metadata."

)

if "description" not in graph[0]:

ignored.append(

"No description found in `ro-crate-metadata.json`."

)

mirpedrol · 2025-04-16T15:45:12Z

nf_core/pipelines/lint/rocrate_readme_sync.py

+    if readme_content != rc_description_graph:
+        # If the --fix flag is set, you could overwrite the RO-Crate description with the README content:
+        if "rocrate_readme_sync" in self.fix:
+            metadata_dict.get("@graph")[0]["description"] = readme_content


Suggested change

metadata_dict.get("@graph")[0]["description"] = readme_content

if "description" not in graph[0]:

graph[0]["description"] = readme_content

fixed.append(

"Fixed: add the same description from `README.md` to the RO-Crate metadata."

)

return {"passed": passed, "failed": failed, "ignored": ignored, "fixed": fixed, "could_fix": could_fix}

metadata_dict.get("@graph")[0]["description"] = readme_content

tests/pipelines/lint/test_rocrate_readme_sync.py

mirpedrol · 2025-04-16T15:55:15Z

tests/pipelines/lint/test_rocrate_readme_sync.py

+        with open(json_path, "w") as f:
+            f.write('{ "@graph": [{"description": "This is a test script"}] }')


To parse the file as a JSON you can use the library json as you did in nf_core/pipelines/lint/rocrate_readme_sync.py. This code is untested so make sure that I didn't make any mistake before accepting the suggestion 😄

Suggested change

with open(json_path, "w") as f:

f.write('{ "@graph": [{"description": "This is a test script"}] }')

with open(json_path, "rw") as f:

try:

rocrate = json.load(fh)

except json.JSONDecodeError as e:

raise UserWarning(f"Unable to load JSON file '{json_path}' due to error {e}")

rocrate["@graph"][0]["description"] = "This is a test script"

json.dump(rocrate, fh, indent=4)

Thanks Julia!
The tests are still failing. it seems like in the git actions, the rocrate created a metadata file without the description key.In this case, when the --fix is not provided, the tests will anyway fail all the ignored tests.

Co-authored-by: Júlia Mir Pedrol <[email protected]>

ningyuxin1999 added 2 commits March 17, 2025 21:34

rocrate readme sync

f9ab845

merge branch 'main' into dev-sync

e2e5285

ningyuxin1999 requested review from ewels, mashehu, mirpedrol and JulianFlesch March 17, 2025 20:51

ningyuxin1999 self-assigned this Mar 17, 2025

ningyuxin1999 and others added 15 commits March 24, 2025 11:22

final test done

a728a47

Merge branch 'dev' into dev-sync

7b23550

failing tests

4989b14

Merge remote-tracking branch 'upstream/dev' into dev-sync

51e183c

Merge branch 'dev-sync' of https://github.com/ningyuxin1999/tools int…

70cc699

…o dev-sync

fix the script

c76713d

api update

409f75c

prettier

e76ec56

Merge branch 'dev' into dev-sync

eb0dd5b

test

bcd16e1

format

36a75ea

testscript

0e04e7f

raise error when script fails

21c5c8d

Merge branch 'dev' into dev-sync

3f98c1d

json error

338f2c1

ningyuxin1999 added this to Hackathon March 2025 Mar 25, 2025

github-project-automation bot moved this to To do in Hackathon March 2025 Mar 25, 2025

mirpedrol reviewed Mar 25, 2025

View reviewed changes

ningyuxin1999 and others added 5 commits March 25, 2025 15:40

Update nf_core/pipelines/lint/pipeline_todos.py

cd83b0d

Co-authored-by: Júlia Mir Pedrol <[email protected]>

Update nf_core/pipelines/lint/rocrate_readme_sync.py

1699e58

Co-authored-by: Júlia Mir Pedrol <[email protected]>

Update nf_core/pipelines/lint/rocrate_readme_sync.py

7949ccf

Co-authored-by: Júlia Mir Pedrol <[email protected]>

Update nf_core/pipelines/lint/rocrate_readme_sync.py

1f3d232

Co-authored-by: Júlia Mir Pedrol <[email protected]>

comments

8a7791c

ningyuxin1999 and others added 8 commits April 10, 2025 14:38

Update nf_core/pipelines/lint/rocrate_readme_sync.py

266d11a

Co-authored-by: Júlia Mir Pedrol <[email protected]>

add non-empty rocrate

1432568

name

1ebfa9a

option

6aff97e

fixignore

7fc6fda

fixlabel

91e6914

fix

ea586e7

add test to utils

5358d84

mashehu reviewed Apr 15, 2025

View reviewed changes

Merge branch 'dev' into dev-sync

7c29f9b

mirpedrol reviewed Apr 15, 2025

View reviewed changes

ningyuxin1999 and others added 6 commits April 16, 2025 14:14

Update nf_core/pipelines/lint/rocrate_readme_sync.py

43df8a3

Co-authored-by: Júlia Mir Pedrol <[email protected]>

Update nf_core/pipelines/lint/rocrate_readme_sync.py

644553a

Co-authored-by: Júlia Mir Pedrol <[email protected]>

Update nf_core/pipelines/lint/rocrate_readme_sync.py

43f030d

Co-authored-by: Júlia Mir Pedrol <[email protected]>

Update nf_core/pipelines/lint/rocrate_readme_sync.py

ec9cbc4

Co-authored-by: Júlia Mir Pedrol <[email protected]>

Update nf_core/pipelines/lint/rocrate_readme_sync.py

5e6eb12

Co-authored-by: Júlia Mir Pedrol <[email protected]>

Merge branch 'dev' into dev-sync

108f659

ningyuxin1999 added 2 commits April 16, 2025 14:59

add test, remove --fix in git action

146e68b

add description directly when there is no description key found

7e26eec

mirpedrol reviewed Apr 16, 2025

View reviewed changes

ningyuxin1999 and others added 5 commits April 17, 2025 11:34

restructure

1ffc229

test

4df9af0

Merge branch 'dev' into dev-sync

4713d10

Update tests/pipelines/lint/test_rocrate_readme_sync.py

975f834

Co-authored-by: Júlia Mir Pedrol <[email protected]>

json load in test scripts

5d8045a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RO-crate content appear in the nf-core lint warning if there is a TODO #3493

RO-crate content appear in the nf-core lint warning if there is a TODO #3493

ningyuxin1999 commented Mar 17, 2025 •

edited

Loading

mirpedrol left a comment

ningyuxin1999 commented Apr 15, 2025

mashehu Apr 15, 2025

mashehu left a comment •

edited

Loading

codecov bot commented Apr 15, 2025 •

edited

Loading

mashehu commented Apr 15, 2025

mirpedrol left a comment

mirpedrol Apr 15, 2025

ningyuxin1999 Apr 16, 2025

mirpedrol Apr 15, 2025

mirpedrol Apr 15, 2025

ningyuxin1999 commented Apr 16, 2025

mirpedrol left a comment

mirpedrol Apr 16, 2025

mirpedrol Apr 16, 2025

mirpedrol Apr 16, 2025

ningyuxin1999 Apr 17, 2025

		with open(json_path, "w") as f:
		f.write('{ "@graph": [{"description": "This is a test script"}] }')

-        with open(json_path, "w") as f:
-            f.write('{ "@graph": [{"description": "This is a test script"}] }')
+        with open(json_path, "rw") as f:
+            try:
+                rocrate = json.load(fh)
+            except json.JSONDecodeError as e:
+                raise UserWarning(f"Unable to load JSON file '{json_path}' due to error {e}")
+            rocrate["@graph"][0]["description"] = "This is a test script"
+            json.dump(rocrate, fh, indent=4)

RO-crate content appear in the nf-core lint warning if there is a TODO #3493

Are you sure you want to change the base?

RO-crate content appear in the nf-core lint warning if there is a TODO #3493

Conversation

ningyuxin1999 commented Mar 17, 2025 • edited Loading

Changes:

PR checklist

mirpedrol left a comment

Choose a reason for hiding this comment

ningyuxin1999 commented Apr 15, 2025

Choose a reason for hiding this comment

mashehu left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Apr 15, 2025 • edited Loading

Codecov Report

mashehu commented Apr 15, 2025

mirpedrol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ningyuxin1999 commented Apr 16, 2025

mirpedrol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ningyuxin1999 commented Mar 17, 2025 •

edited

Loading

mashehu left a comment •

edited

Loading

codecov bot commented Apr 15, 2025 •

edited

Loading