Feature #1225 generate pdf by maartenhammer · Pull Request #1475 · ExploreASL/ExploreASL

maartenhammer · 2023-08-14T07:48:19Z

Linked issue

How to test

You can run the pdf generator using [x, config] = xASL_qc_GenerateReport(x[, subject]), with x being the ExploreASL s struct, if no subject is provided, it will default to the subject defined in x.SUBJECT.
A Small manual has been written that goes over the basics to creating a config report for the layout.
If no configuration file is found in <dataroot>/Derivatives/ExploreASL it will use the default in the exploreASL root directory.

Manual should be moved to the docs before merging!

HenkMutsaerts

Very nice work! Some minor stuff, that are mainly about code, header, and comment consistencies, trying to keep everything readable. You currently have a lot of subfunctions where variables change names (e.g., "input") and depending on the content of the variable a different action will be triggered by a single function, that may not help readability and debugging. I would always go for simplicity, like you nicely do sometimes with things like

switch type
A -> do this subfunction
B -> do this subfunction

instead of doing the same subfunction for both A and B but inside the subfunction determining to what other subfunction A or B go

Another important issue: you use imshow and perhaps other functions from the Matlab image processing toolbox. Jan and I have put a lot of effort in not having any toolbox dependencies outside Matlab itself. This will ensure that most users can use ExploreASL (also those that didn't pay for the toolbox) and that the docker container stays lean.

Functions/Administration/xASL_adm_GeneratePDFConfig.m

Functions/QualityControl/xASL_qc_GenerateReport.m

jan-petr

Some minor changes needed.

Functions/Administration/xASL_adm_GeneratePDFConfig.m

Functions/Administration/xASL_adm_LoadPDFConfig.m

Functions/QualityControl/xASL_qc_GenerateReport.m

HenkMutsaerts · 2023-09-15T09:34:40Z

Just tested this:

[x, config] = xASL_qc_GenerateReport(x[, subject]) as you state above is not working, there is only a single x output.

Then I saw 2 minor bugs which I fixed.

Finally, the current config gives an example, can you replace the default config with one that creates a similar PDF as we had? Then we can improve it from there, but then at least I can already check how it works.

Also, now if there are any pre-existing xASL reports, it will delete them; this behavior should be disabled. Only, if there already is a PDF existing with an identical filename, it should perhaps rename the new file (I wouldn't touch the old/pre-existing file).

Also, this function now is dependent on x.P, so if a filename exists but is not defined in x.P, it will not be printed. I would instead just use relative filenames, not x.P fields (those fields are only useful within ExploreASL).

maartenhammer · 2023-10-20T13:20:07Z

Just tested this:

[x, config] = xASL_qc_GenerateReport(x[, subject]) as you state above is not working, there is only a single x output.

Then I saw 2 minor bugs which I fixed.

Finally, the current config gives an example, can you replace the default config with one that creates a similar PDF as we had? Then we can improve it from there, but then at least I can already check how it works.

Also, now if there are any pre-existing xASL reports, it will delete them; this behavior should be disabled. Only, if there already is a PDF existing with an identical filename, it should perhaps rename the new file (I wouldn't touch the old/pre-existing file).

Also, this function now is dependent on x.P, so if a filename exists but is not defined in x.P, it will not be printed. I would instead just use relative filenames, not x.P fields (those fields are only useful within ExploreASL).

Excellent, the changes added by you seem to improve stability when parameters are missing, as well as some householding data.

I will also improve this by including the definitions here as default for units and names if no alternatives are given.

jan-petr

Good work. No further comments.

maartenhammer · 2023-11-22T15:47:10Z

Last conversations have been handled:
Translation layer added that translates ExploreASL Legacy to BIDS when turned on
Settings has been renamed to TextSettings for clarity
Image has been renamed to Image2D
scan has been renamed to Image3D
A secondary categorizer has been added (instead of a json item being Type = image2D, we now have category = "content" , type = "image".
This is to distinguish metadata json-objects (pages, blocks, settings) and content json objects(like image2d, image3d, text, QCValues).
Added translation layer to always automatically translate some of the qc parameters here. Not all have been filled in, discuss with either Jan/Henk Jan which should be filled in.
Changed default print to look more like the example given in figure 3 of the ExploreASL paper .
New default now looks like this

Edit:
Discussion with Henk-Jan on thursday.
Possible additions:
Pdfconfig json generator that reads x.output and populates a PDFconfig.json instead of using a default template in Functions->Quality Controll, add it to wrapper.
Add text elipses (...) when qc_param exceeds certain length. (added)

maartenhammer · 2023-11-24T14:37:35Z

While adding text wrapping I've noticed that xASL_num2str does not convert logical to str, which the matlab native num2str does.
A minor change to xASL_num2str is added in 3e1290d which fixes this, as I assume this is a bug instead of intended behaviour. (Discussed with Jan over whatsapp).

jan-petr

I am merely blocking the merging, because some commit messages need fixing ;)

HenkMutsaerts

Nice work! Some questions as we discussed, to keep the current functionality of generating the PDF dependent on what was analyzed in ExploreASL.

E.g, if you have analyzed 2 ASL runs, you want to show 2 ASL runs. If you have analyzed only T1w and FLAIR, you want to only show them.

So while there might be a static part of the template/settings file, it would be nice if these things can also be generated dependently.

Functions/QualityControl/qc_translation.tsv

Functions/QualityControl/templateConfigReportPDF.json

HenkMutsaerts

Some minor comments, and a bug while testing ExploreASL's TestDataSet

Functions/Administration/xASL_adm_LoadPdfConfig.m

Functions/Administration/xASL_adm_GeneratePdfConfig.m

Functions/QualityControl/xASL_qc_GenerateReport.m

Functions/QualityControl/xASL_qc_ParsePdfConfig.m

HenkMutsaerts

Few additional things.

Functions/QualityControl/templateConfigReportPDF.json

Functions/QualityControl/templateConfigReportPDFAlternative.json

maartenhammer · 2023-12-21T16:01:33Z

Two force pushed to incorporate two minor appends to the last commit.

HenkMutsaerts

My test worked fine. I have added some wishes to the follow-up issue #1476 (in which we actually implement this in ExploreASL).

… and variables to xASL_qc_ParsePdfConfig

…prove specifity of subfunctions

…Legacy

…arification

…mentation

…Text)

…Images)

…qc_values)

…qc_images)

…eanup of code

…ple values

maartenhammer added the feature New feature, enhancement or request label Aug 14, 2023

maartenhammer added this to the Release 1.11.0 milestone Aug 14, 2023

maartenhammer requested review from HenkMutsaerts and jan-petr August 14, 2023 07:48

maartenhammer assigned HenkMutsaerts, jan-petr and maartenhammer Aug 14, 2023

jan-petr linked an issue Aug 15, 2023 that may be closed by this pull request

PDF Report restart #1225

Closed

21 tasks

jan-petr removed this from the Release 1.11.0 milestone Aug 15, 2023

HenkMutsaerts requested changes Aug 15, 2023

View reviewed changes

jan-petr requested changes Aug 16, 2023

View reviewed changes

HenkMutsaerts unassigned HenkMutsaerts and jan-petr Oct 19, 2023

HenkMutsaerts removed a link to an issue Oct 19, 2023

PDF Report restart #1225

Closed

21 tasks

HenkMutsaerts linked an issue Oct 19, 2023 that may be closed by this pull request

PDF Report restart #1225

Closed

21 tasks

jan-petr approved these changes Oct 27, 2023

View reviewed changes

jan-petr requested changes Nov 29, 2023

View reviewed changes

jan-petr requested a review from HenkMutsaerts November 29, 2023 20:25

HenkMutsaerts requested changes Nov 30, 2023

View reviewed changes

Functions/QualityControl/qc_translation.tsv Outdated Show resolved Hide resolved

Functions/QualityControl/templateConfigReportPDF.json Outdated Show resolved Hide resolved

jan-petr requested a review from HenkMutsaerts December 4, 2023 09:35

HenkMutsaerts requested changes Dec 19, 2023

View reviewed changes

HenkMutsaerts requested changes Dec 20, 2023

View reviewed changes

Functions/QualityControl/templateConfigReportPDF.json Outdated Show resolved Hide resolved

Functions/QualityControl/templateConfigReportPDFAlternative.json Show resolved Hide resolved

maartenhammer force-pushed the feature-#1225-GeneratePDF branch 2 times, most recently from 60a2806 to f218d7a Compare December 21, 2023 15:59

HenkMutsaerts approved these changes Dec 22, 2023

View reviewed changes

maartenhammer and others added 27 commits December 22, 2023 18:02

#1225 xASL_qc_GenerateReport fixed spacing after punctuation

05297de

#1225 renamed subfunction from xASL_sub_ to xASL_qc_GenerateReport_sub_

8773c94

#1225 Changed xASL_qc_GenerateReport to wrapper, renamed subfunctions…

7febc1a

… and variables to xASL_qc_ParsePdfConfig

#1225 xASL_qc_ParsePdfConfig renamed json/input to <type>Struct to im…

103484f

…prove specifity of subfunctions

#1225 xASL_qc_ParsePdfConfig added more comments

cc944fb

#1225 xASL_qc_Parse_PdfConfig added comments to subfunctions.

6303f96

#1225 xASL_qc_parsePdfConfig comment changes

53ed0e2

#1225 xASL_qc_GenerateReport: bugfix x.SUBJECT

3e91355

#1225 xASL_qc_GenerateReport: bugfix x.SESSION

56c170e

#1225 xASL_qc_GenerateReport: bugfix x.P

3bd4f6c

#1225 xASL_qc_GenerateReport: trying tracking

0a9bf3b

#1225 xASL_qc_GenerateReport: disable figure in cli

ffef028

#1225 xASL_qc_ParsePdfConfig: Header fix

3966b03

#1225 GeneratePDF_Report added translation function BIDS->ExploreASL …

dc4e4ba

…Legacy

#1225 Generate PDF report changed 'settings' to 'textSettings' for cl…

ca7b428

…arification

#1225 xASL_qc_GenerateReport added category/type distinction

4c25a25

#1225 GeneratePDF added qc default translations and rudimentory imple…

2e81b39

…mentation

#1225 Changed default PDF report to look the same as old PDF Report (…

b9b762e

…Text)

#1225 Changed default PDF report to look the same as old PDF Report (…

24a58d2

…Images)

#1225 xASL_generateReport added string width limiting and ellipsis

54c3023

#1225 xASL_num2str fixed bug prevention translation of logical variables

ec69c7b

#1225 Move PDFGenerator.md to docu

92fc66b

#1225 Added script that automatically generates a PDF report config (…

fe85dde

…qc_values)

#1225 Added script that automatically generates a PDF report config (…

62fac23

…qc_values)

#1225 Added script that automatically generates a PDF report config (…

9fa7466

…qc_images)

#1225 Generate PDF report moving functions, addition of comments, cl…

9512370

…eanup of code

#1225 Generate Report Fixed bug when quality paramteres contain multi…

c394f5e

…ple values

jan-petr force-pushed the feature-#1225-GeneratePDF branch from 51be2f0 to c394f5e Compare December 22, 2023 17:03

jan-petr merged commit c394f5e into develop Dec 22, 2023

jan-petr deleted the feature-#1225-GeneratePDF branch December 22, 2023 17:03

Conversation

maartenhammer commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Linked issue

How to test

Uh oh!

HenkMutsaerts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HenkMutsaerts commented Sep 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maartenhammer commented Oct 20, 2023

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

maartenhammer commented Nov 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maartenhammer commented Nov 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

HenkMutsaerts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

HenkMutsaerts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HenkMutsaerts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maartenhammer commented Dec 21, 2023

Uh oh!

HenkMutsaerts left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

maartenhammer commented Aug 14, 2023 •

edited

Loading

HenkMutsaerts commented Sep 15, 2023 •

edited

Loading

maartenhammer commented Nov 22, 2023 •

edited

Loading

maartenhammer commented Nov 24, 2023 •

edited

Loading