Scout: PDF Delivery report is truncated

[hassanfa]

All the coverage qc reports that are delivered to Scout through deliver_report functionality are truncated and data from last column is unreadable. HTML is readable: https://scout.scilifelab.se/cust107/2017-00600-reprep

Reproduce:

1. Select a cancer case with a coverage qc report:
   

1. Select PDF report:
   

[northwestwitch]

Ah it took me a while to understand that I should look at the delivery report, I was looking at the coverage report and it was fine 😄 . I'll try to fix it ASAP

[northwestwitch]

One thing I don't understand here is why the report named "Coverage and QC report", that you rightfully named Coverage QC report in this issue is loaded with the key delivery_report instead of coverage_qc_report in Scout. I think this is an unrelated problem that occurred upstream. If it's not a big deal it is fine, but otherwise we could fix it in 2 ways:

• Fix the process that creates the cancer case config file (cg?)
• Remove the coverage_qc_report key option from scout and have all cancer cases loaded with this report as delivery_report (as it is now)

[hassanfa]

cg dev don't want to use coverage_qc_report for some reason that I didn't really understand. We can move this issue to cg and properly use coverage_qc_report?

In cancer analysis we will eventually use coverage_qc_report and delivery_report.

[northwestwitch]

If as you say we're going to need both keys in cancer analysis then this should be addressed in cg ASAP I think. I can open an issue there and leave this one open here as the ugly formatting of the PDF is not fixed yet!

[hassanfa]

Are you sure you want to fix PDF render based on a report that has different styling? Will this affect rest of the delivery reports?

[northwestwitch]

I've tried changing the key and it looks ugly also as a coverage_qc_report, so yes! I've opened this issue in cg

[northwestwitch]

It will require testing of the other reports as well, but I'll mostly fix the rendering of the document under coverage_qc_report

[dnil]

Hi both! At the clinic today, but three things:

• the cancer qc report from Balsamic is currently the only delivery report available. Hence it will be used as such until there is a separation. We have talked at length with @Mropat @moonso among others, who strongly prefer it this way: I gave in, and this is very workable for now. Scout can use both, and the key as such will be useful later. Please sync with Balsamic dev for the cg issue ok, so there are actually two different files to upload first?
• Do you remember (@hassanfa) if you ever saw it rendered ok as pdf? I found an old temp dl:ed pdf from feb which also had the same issue.
• The pdf rendering library is sensitive to the html, and especially css, so what you rather may want to do is simplify esp some of the in-doc css from the html already in Balsamic.

[northwestwitch]

Quick comment. I just don't see the point in naming something in the wrong way, especially knowing that it will eventually change in the future..

[moonso]

If there is a delivery report and a coverage qc report we should of course upload both. We just need to know what tags to fetch the files from in Housekeeper.

Quick comment. I just don't see the point in naming something in the wrong way, especially knowing that it will eventually change in the future..

there has been some confusion around this topic that is the answer to this question I suppose.

I can change so that the existing report get's uploaded as coverage_qc_report now.

[dnil]

I agree in principle. It is named correctly in scout and balsamic. It works in CG as it is now, and there was a valid point from @moonso and @Mropat that the coverage_qc_report currently looks and functions very much like the rd delivery_report.

[hassanfa]

Hi both! At the clinic today, but three things:

• the cancer qc report from Balsamic is currently the only delivery report available. Hence it will be used as such until there is a separation. We have talked at length with @Mropat @moonso among others, who strongly prefer it this way: I gave in, and this is very workable for now. Scout can use both, and the key as such will be useful later. Please sync with Balsamic dev for the cg issue ok, so there are actually two different files to upload first?

Eventually we will have two different files yes. I don't think delivery report will replace coverage qc report. They serve different purpose.

• Do you remember (@hassanfa) if you ever saw it rendered ok as pdf? I found an old temp dl:ed pdf from feb which also had the same issue.

No I only checked coverage_qc_report in Scout to render properly.

• The pdf rendering library is sensitive to the html, and especially css, so what you rather may want to do is simplify esp some of the in-doc css from the html already in Balsamic.

I'm worried about the same thing as well.

[moonso]

What is confusing is that the coverage_qc_report has the tag delivery_report in housekeeper. So when there is another report available (maybe already now?) I'm not sure how to handle these tags. Should we rename the tags for the old report? If we keep the tag delivery_report for the coverage_qc_report in housekeeper, what tag should we need for the delivery_report etc etc.

On a side note

cg dev don't want to use coverage_qc_report for some reason that I didn't really understand. We can move this issue to cg and properly use coverage_qc_report?

Please stop implying that people don't listen to you @hassanfa , we are all trying to solve this in the best way possible. In this case, just like @dnil says, the report was very similar to the delivery report so it made sense to reuse that field until there are multiple reports available.

[dnil]

• Do you remember (@hassanfa) if you ever saw it rendered ok as pdf? I found an old temp dl:ed pdf from feb which also had the same issue.

No I only checked coverage_qc_report in Scout to render properly.

But they render via very similar looking code, so should be the same. If you saw at an ok pdf report there, it may "just" be some library update on the server. But lets see.

[hassanfa]

I agree in principle. It is named correctly in scout and balsamic. It works in CG as it is now, and there was a valid point from @moonso and @Mropat that the coverage_qc_report currently looks and functions very much like the rd delivery_report.

Well it is not delivery report only containing coverage qc report. It'll be expanded to cover more QC information.

[hassanfa]

Please stop implying that people don't listen to you @hassanfa , we are all trying to solve this in the best way possible.

I didn't imply anything. From DN's comment I see what's the reason now.

[northwestwitch]

Keeping the discussion focused on the issue (please appreciate the effort! 😆 ) , we're still using that old version of flask_weasyprint, could be that!

[dnil]

What is confusing is that the coverage_qc_report has the tag delivery_report in housekeeper. So when there is another report available (maybe already now?) I'm not sure how to handle these tags. Should we rename the tags for the old report? If we keep the tag delivery_report for the coverage_qc_report in housekeeper, what tag should we need for the delivery_report etc etc.

Yeah, we actually had coverage_qc_report as the tag in the initial PR. No grudges here obviously. 😉 I don't think it is a big issue; as said, they are similar enough, and the content of a given report may change over time.

[moonso]

Ok that sounds good. What is the conclusion? Do you want me to change so that the current report gets uploaded with the coverage_qc_report field or keep uploading it with the delivery_report field as it is now?

[hassanfa]

Anyway, if the CSS layout is not proper in the report I can adopt it to the PDF renderer. QC report has a fixed table length, alternatively I can transpose QC report table to be vertical.

[dnil]

Ok that sounds good. What is the conclusion? Do you want me to change so that the current report gets uploaded with the coverage_qc_report field or keep uploading it with the delivery_report field as it is now?

Potato, potato? My humble suggestion would be to wait until Balsamic produces two files, then switch over in CG/Hermes at that point.

[dnil]

(Or until Chanjo for cancer is operational, then CG can make the delivery report, just like for RD..)

[northwestwitch]

Anyway, if the CSS layout is not proper in the report I can adopt it to the PDF renderer. QC report has a fixed table length, alternatively I can transpose QC report table to be vertical.

This is likely the cause of the problem in rendering, the orientation of the PDF. The rare case delivery report is portrait and the cancer one panorama?

[hassanfa]

Report template is here: https://github.com/Clinical-Genomics/BALSAMIC/blob/master/BALSAMIC/assets/report_template/balsamic_report.html Maybe maxwidth is the problem.

If the change in Scout is too specific and limited to BALSAMIC, then I think it is better to change it BALSAMIC.

[moonso]

Changed here I'll merge that today so you know

[northwestwitch]

@hassanfa if the cancer report will be under coverage_qc_report I can try to see if it can be rendered in landscape orientation by default. I'll let you know how it goes!

[dnil]

Also in the template, the dl and dt width / margin fixes may need to be relaxed / changed? That does not seem to be a page orientation issue. In general, I would advise against this type of fixed sizes for everything, and let the engine render, but if some is fixed, it may be wise just to fix the remaining things..

[hassanfa]

I see. I'll check later what I can do. Also: https://www.bram.us/wordpress/wp-content/uploads/2017/01/css-is-awesome.jpg

[dnil]

That! Precisely that!

[northwestwitch]

I've tested a bit following some suggestions in threads but with no success. I think it can be fixed by changing the css style

[hassanfa]

cg PR to use coverage_qc tag and landscape doesn't help either, am I right?

[northwestwitch]

I tested on the endpoint returning coverage_qc_report field.. No, but I can give it another try later

[hassanfa]

I will look into this early next week, no worries.