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

Images in output.md not rendered when converted to html #391

Closed
drpatelh opened this issue Sep 24, 2019 · 15 comments
Closed

Images in output.md not rendered when converted to html #391

drpatelh opened this issue Sep 24, 2019 · 15 comments
Labels
documentation template nf-core pipeline/component template

Comments

@drpatelh
Copy link
Member

drpatelh commented Sep 24, 2019

The template contains a process called output_documentation that converts the pipeline's output documentation i.e. docs/output.md to html format in order to be stored with the results. However, if there are images embedded or linked in this document then these wont be rendered properly and so the links just appear to be broken. Im wondering whether using absolute instead of relative paths will solve this... 🤔

@drpatelh drpatelh added documentation template nf-core pipeline/component template labels Sep 24, 2019
@drpatelh drpatelh changed the title Images in output.md not copied across when converted to html Images in output.md not rendered when converted to html Sep 24, 2019
@ewels
Copy link
Member

ewels commented Oct 4, 2019

Good spot! Absolute paths is one way, or we could also copy the images directory to the results folder. The best way would be to base64 encode the images so that they are included in the HTML file.

@ewels
Copy link
Member

ewels commented Oct 4, 2019

To be honest, it wouldn't be a terrible idea to rewrite this functionality. I don't love that it's done with an R script, as it means that every pipeline has to have R in the container. Could be better to have it in a Python script, as we're using conda..

@drpatelh
Copy link
Member Author

drpatelh commented Oct 4, 2019

Issues with needing R aside (which I agree isn't ideal for a single package), I'm going to go one step further and ask if we actually need to generate this file at all? It's a duplicate of what's already available online so why not just use/point to that one exclusively for results documentation?

@ewels
Copy link
Member

ewels commented Oct 4, 2019

Yes, the same question passed through my mind to be honest. It is a relic of the early days of the NGI pipelines really. But I think that is probably is still useful in our use case - specifically when pipeline results are delivered to a user who has no knowledge (or interest) of the pipeline that generated those results.

I guess we could just link to the online docs though. Maybe a shortcut file?? Remember those? 😆

output_file_documentation.url:

[InternetShortcut]
URL=https://nf-co.re/atacseq/docs/output

@apeltzer
Copy link
Member

apeltzer commented Oct 4, 2019

Good point @drpatelh! If we could simply link to whats online I think that's the best way to do it and also is anyways the same piece of information.

@drpatelh
Copy link
Member Author

drpatelh commented Oct 4, 2019

I guess the only real problem is that output.md will change over time and so we will need a more dynamic way of referring to the docs for that particular pipeline release. At the moment, thats not a problem because it will be generated from the docs at the time.

@maxulysse
Copy link
Member

We can always refer to a permalink of the docs

@ewels
Copy link
Member

ewels commented Oct 14, 2019

I just tested the example output_file_documentation.url file above and it actually worked pretty nicely on my mac..

Referring to specific versions won't work on the nf-core website, but pinning to a commit hash on GitHub.com should be fine..

@ewels
Copy link
Member

ewels commented Oct 14, 2019

Looks like you can use tag (release) names directly in the GitHub URLs too, so we don't have to worry about fetching commit hashes: https://github.com/nf-core/atacseq/blob/1.0.0/docs/output.md

@ewels
Copy link
Member

ewels commented Feb 6, 2020

I have now rewritten the markdown conversion in Python and this includes base64 including images.

See #501

@ewels ewels closed this as completed Feb 6, 2020
@drpatelh
Copy link
Member Author

The conversion in Python is working great now but still no images being rendered in the output html 😓

@drpatelh
Copy link
Member Author

Thanks to @svarona for this proposed fix:
nf-core/viralrecon@c25d62a

@ewels
Copy link
Member

ewels commented Apr 17, 2020

haha, is that it? That the images weren't being staged in the work directory? Of course! Nothing to do with the script itself - no wonder I couldn't replicate the problem 🤦

Nice work @svarona!

@apeltzer
Copy link
Member

Oh my 🤦‍♂️ And everyone was super crazy about "what is wrong there 🤔 " .... thanks a lot @svarona !

@apeltzer apeltzer mentioned this issue Apr 18, 2020
5 tasks
@apeltzer
Copy link
Member

Fix in #600

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation template nf-core pipeline/component template
Projects
None yet
Development

No branches or pull requests

4 participants