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

Add sample metadata tables to reports #769

Merged
merged 9 commits into from
Jul 8, 2024

Conversation

allyhawkins
Copy link
Member

Closes #765

This PR adds a function to create a sample metadata table to report_functions.Rmd and then uses that function in both the main QC report and cell type report to create the sample metadata table. The table is shown for any non-multiplexed samples. Otherwise, we have a nice little blue box that says the metadata will not be displayed since the sample is not demultiplexed. I took the wording from the merge report where we also don't show sample metadata.

The table is formatted in the same way we have our other tables and I was able to successfully add links for the ontology IDs 🎉. I also accounted for missing ontology IDs and only provide a link if an ID was provided.

In the main report I adjusted a few headers to account for adding in this table.

Questions for reviewers:

  • Are we okay with the values that I chose to include? Things I am not displaying are project ID, library ID, submitter ID, submitter, participant ID, or self reported ethnicity term id.
  • How do we feel about the location of the table?
  • I added some explanatory text above the table, but I'm not totally convinced we need it.

Also, I've been seeing the following error for this table, but I'm not sure how to fix it. The table is still printed, but if we can remove this message that's probably best?

Warning message:
In yaml::yaml.load(yaml, handlers = knit_params_handlers(evaluate = evaluate), :
an error occurred when handling type 'r'; using default handler

Here's a zip with copies of the updated QC and cell type reports for a multiplexed and non-multiplexed sample:
updated_reports.zip

@jashapiro
Copy link
Member

Also, I've been seeing the following error for this table, but I'm not sure how to fix it. The table is still printed, but if we can remove this message that's probably best?

Warning message:
In yaml::yaml.load(yaml, handlers = knit_params_handlers(evaluate = evaluate), :
an error occurred when handling type 'r'; using default handler

Can you narrow down when you are seeing this warning? I don't see it in any of the reports you sent, so is it only with some samples?

@allyhawkins
Copy link
Member Author

Can you narrow down when you are seeing this warning? I don't see it in any of the reports you sent, so is it only with some samples?

It doesn't print out in the reports, but only when I run the chunk that has the new metadata table locally or when I render the report it prints out in R.

Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

This looks fine overall. I don't have great ideas about the warning you are seeing, but you might be able to narrow it down by turning on debugging with debug(print_sample_metadata) and running the chunk. That should allow you to find exactly where the warning is coming from.

I have some suspicion it may because some values are encoded by text_spec and some are not, but that is really just a guess.

templates/qc_report/utils/report_functions.rmd Outdated Show resolved Hide resolved

The below table summarizes clinical metadata for the sample associated with this library.
Blue hyperlinks are present for any terms with an ontology term identifier associated with the displayed human readable value.
These links will direct you to the [`Ontobee`](https://ontobee.org/) page for that ontology term identifier.
Copy link
Member

Choose a reason for hiding this comment

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

This link isn't always to Ontobee, so you probably want something a bit more general.

allyhawkins and others added 3 commits July 3, 2024 15:55
Co-authored-by: Joshua Shapiro <josh.shapiro@ccdatalab.org>
@allyhawkins
Copy link
Member Author

I don't have great ideas about the warning you are seeing, but you might be able to narrow it down by turning on debugging with debug(print_sample_metadata) and running the chunk. That should allow you to find exactly where the warning is coming from.

I think it might be something with my Rstudio? I get the warning for any chunk that I run, but the chunk runs successfully. I found a similar issue here. I could be way off, but I also ran the code in the console and don't see any warnings. So it's definitely something with the notebook itself. I also don't think it's a huge deal since rendering it works as expected?

I also incorporated your edits and removed the Ontobee link so this should be ready for another look.

Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

LGTM, but probably worth a code styling run before merging.

Comment on lines 400 to 405
<div class=\"alert alert-info\">

This library is multiplexed and contains data from more than one sample.
Demultiplexing has not been performed, so sample metadata will not be displayed.
</div>
"))
Copy link
Member

Choose a reason for hiding this comment

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

Indentation

Suggested change
<div class=\"alert alert-info\">
This library is multiplexed and contains data from more than one sample.
Demultiplexing has not been performed, so sample metadata will not be displayed.
</div>
"))
<div class=\"alert alert-info\">
This library is multiplexed and contains data from more than one sample.
Demultiplexing has not been performed, so sample metadata will not be displayed.
</div>
"))

Comment on lines 243 to 251
knitr::asis_output(
glue::glue("
<div class=\"alert alert-info\">

This library is multiplexed and contains data from more than one sample.
Demultiplexing has not been performed, so sample metadata will not be displayed.
</div>
")
)
Copy link
Member

Choose a reason for hiding this comment

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

Formatting here too... Maybe give this a run through styler?

@allyhawkins allyhawkins merged commit 477f827 into development Jul 8, 2024
4 checks passed
@allyhawkins allyhawkins deleted the allyhawkins/sample-metadata-to-reports branch July 8, 2024 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants