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

Implement tool markdown reports. #19054

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

jmchilton
Copy link
Member

@jmchilton jmchilton commented Oct 24, 2024

Many tools produce HTML and we do our best to support those but they are less than ideal. We cannot safely trust those HTML files to be rendered after they have been imported - probably even rendering the HTML of "safe tools" will catch up with us at some point. The ability to be able to safely render these Markdown components even after an export and re-import makes them more reproducible IMO. Once implemented, it will be much easier to compose these chunks and embed them into workflow reports also and even in the PDF version of those reports - where we couldn't really do much with HTML at all I believe.

I've included two test tools. One that links to and display the outputs of the tool and a second one that utilizes "extra files" content of the report itself - if the tool would like to included data in the report that shouldn't be a formal output of the tool. I've discovered some limitations around extra files rendering and I've included some bug fixes to get the existing components to work. There is still work to be done around these components but I think that is independent of this task.

The next steps include documenting which directives are available to tools (some don't make sense like workflow_license), better reporting when there are errors, and allowing the embed of these outputs into workflow and page reports after the fact.

xref #19039

Screenshot 2024-10-24 at 1 19 08 PM

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • Instructions for manual testing are as follows:
    1. Play with the included test tools and see what works and what doesn't.

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool idea. @neoformit maybe something for the Alphafold tool?

@neoformit
Copy link
Collaborator

Very cool idea. @neoformit maybe something for the Alphafold tool?

Yep, this could be a nice addition to the HTML output. We met last week with the structural biology community who were mostly interested in collating output metrics/plots for interpretation of model confidence. An Md report would make a nice job of pulling that together.

@jmchilton jmchilton marked this pull request as ready for review November 12, 2024 21:34
@github-actions github-actions bot added this to the 25.0 milestone Nov 12, 2024
dataset_instance = self.hda_manager.get_accessible(dataset_id, trans.user)
self.hda_manager.ensure_dataset_on_disk(trans, dataset_instance)
file_path = trans.app.object_store.get_filename(dataset_instance.dataset)
raw_content = open(file_path).read()
Copy link
Member

@mvdbeek mvdbeek Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you limit this to something like 10KB ? Also the with handler is a good idea.

Copy link
Member

@mvdbeek mvdbeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool, just a minor tweak to prevent an easy OOM request.

@qchiujunhao
Copy link

It’s probably too late, but I’m wondering if we can release the tool_markdown in version 24.2. Or is there a specific reason to wait for version 25.0? @jmchilton

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants