Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misleading TODO item for "contains" entry in test YAML file. #1082

Closed
charles-plessy opened this issue May 18, 2021 · 2 comments
Closed

Misleading TODO item for "contains" entry in test YAML file. #1082

charles-plessy opened this issue May 18, 2021 · 2 comments
Labels
bug Something isn't working

Comments

@charles-plessy
Copy link
Contributor

charles-plessy commented May 18, 2021

Description of the bug

nf-core modules create-test-yml sometimes inserts the following TODO item:

# TODO nf-core: file md5sum was variable, please replace this text with a string found in the file instead

However, following the instructions results in a borken file, as the contains map need to contain an array.

Expected behaviour

How about either:

  • edit the TODO so that it asks the users to ...replace this text with an array of strings found...
  • or insert the TODO in an array so that replacing the text results in the correct YAML structure, like:
      files:
        - path: path/to/file
           contains:
             - "# TODO nf-core: file md5sum was variable, please replace..."
@charles-plessy charles-plessy added the bug Something isn't working label May 18, 2021
@ewels
Copy link
Member

ewels commented May 18, 2021

Relevant code:

# Compare both test.yml files
for i in range(len(test_files)):
if not test_files[i]["md5sum"] == test_files_repeat[i]["md5sum"]:
test_files[i].pop("md5sum")
test_files[i][
"contains"
] = "# TODO nf-core: file md5sum was variable, please replace this text with a string found in the file instead"

Should be a simple enough change - just wrapping an extra set of [] around the string 👍🏻

Do you fancy putting together a PR @charles-plessy?

charles-plessy added a commit to charles-plessy/tools that referenced this issue May 19, 2021
The `contains:` map needs an array of strings.  By placing the TODO comment between brackets I hope that the user will be more likely to produce a valid file. 

Closes: nf-core#1082
@KevinMenden
Copy link
Contributor

closed by #1089

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants