Allow use of a custom config file #29

adamjtaylor · 2024-02-07T13:10:52Z

HistoQC uses configs to determine which modules are run (eg pen markings, blurry areas, fill small holes etc).

Natively HistoQC's config allows the user to pass either a string matching the name of a built-in config file (eg first or v2.1) or the path to a custom config file (eg folder/my_custom_config.ini).

To enable the use of custom configs in nf-histoqc we need to add some logic to pass a file if a custom config is used or the string if a default config is used.

I thought it best to split this into a new param custom_config that overrides the param config if provided.
It is clunky but there is then logic in both the subworkflow run_histoqc.nf and the module histoqc.nf to check which param is being used and to appropriately stage the custom config file.

adamjtaylor · 2024-02-07T17:07:31Z

I seem to be stuck where this works for a custom config file when --convert is not set, but when it is set it fails

This fails

% nextflow run main.nf --input test_data/test_samplesheet.csv --mag 10 --mpp 0.1 --custom_config config_v2.1_pen.ini --profile local  --convert
N E X T F L O W  ~  version 23.04.4
Launching `main.nf` [romantic_gautier] DSL2 - revision: 5e4f4b36b8
config_ch: /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini
executor >  local (4)
[07/febed0] process > NF_HISTOQC:RUN:CONVERT (2)                       [100%] 2 of 2 ✔
[26/aaa992] process > NF_HISTOQC:RUN:HISTOQC (CMU-1-Small-Region-copy) [  0%] 0 of 2
executor >  local (4)
[07/febed0] process > NF_HISTOQC:RUN:CONVERT (2)                       [100%] 2 of 2 ✔
[76/23286f] process > NF_HISTOQC:RUN:HISTOQC (CMU-1-Small-Region)      [100%] 1 of 1, failed: 1
[-        ] process > NF_HISTOQC:COLLECT:RESULTS                       -
[-        ] process > NF_HISTOQC:COLLECT:TIDY                          -
[-        ] process > NF_HISTOQC:COLLECT:LOGS                          -
ERROR ~ Error executing process > 'NF_HISTOQC:RUN:HISTOQC (CMU-1-Small-Region-copy)'

Caused by:
  Process `NF_HISTOQC:RUN:HISTOQC (CMU-1-Small-Region-copy)` terminated with an error exit status (1)

Command executed:

  echo "Using config: /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini"
  python -m histoqc -c /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini CMU-1-Small-Region-copy_openslide.tiff -o out
  mv out/CMU-1-Small-Region-copy_openslide.tiff/*.png .

Command exit status:
  1

Command output:
  Using config: /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini

Command error:
  WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
  Using config: /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini
  2024-02-07 17:06:39,195 - WARNING - Configuration file /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini assuming to be a template...checking.
  Traceback (most recent call last):
    File "/usr/local/lib/python3.8/runpy.py", line 194, in _run_module_as_main
      return _run_code(code, main_globals, None,
    File "/usr/local/lib/python3.8/runpy.py", line 87, in _run_code
      exec(code, run_globals)
    File "/opt/HistoQC/venv/lib/python3.8/site-packages/histoqc/__main__.py", line 227, in <module>
      sys.exit(main())
    File "/usr/local/lib/python3.8/contextlib.py", line 75, in inner
      return func(*args, **kwds)
    File "/opt/HistoQC/venv/lib/python3.8/site-packages/histoqc/__main__.py", line 82, in main
      config.read_string(read_config_template(args.config))
    File "/opt/HistoQC/venv/lib/python3.8/site-packages/histoqc/config/__init__.py", line 37, in read_config_template
      raise KeyError(f'no configuration template found under key {name!r}')
  KeyError: "no configuration template found under key '/Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini'"

Work dir:
  /Users/ataylor/Documents/projects/htan/nf-histoqc/work/26/aaa992cc7b3c2ca437246ed5721214

Tip: when you have fixed the problem you can continue the execution adding the option `-resume` to the run command line

 -- Check '.nextflow.log' file for details

But this passes

% nextflow run main.nf --input test_data/test_samplesheet.csv --mag 10 --mpp 0.1 --custom_config config_v2.1_pen.ini --profile local           
N E X T F L O W  ~  version 23.04.4
Launching `main.nf` [fervent_kare] DSL2 - revision: 5e4f4b36b8
config_ch: /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini
executor >  local (5)
[ac/993e1c] process > NF_HISTOQC:RUN:HISTOQC (CMU-1-Small-Region)      [100%] 2 of 2 ✔
[18/b3030f] process > NF_HISTOQC:COLLECT:RESULTS                       [100%] 1 of 1 ✔
[46/cceab9] process > NF_HISTOQC:COLLECT:TIDY                          [100%] 1 of 1 ✔
[1d/f1d961] process > NF_HISTOQC:COLLECT:LOGS                          [100%] 1 of 1 ✔

adamjtaylor · 2024-02-07T17:36:40Z

subworkflows/run_histoqc.nf

+    if (params.convert) {
+        CONVERT( images ) 
+        HISTOQC( CONVERT.out, config_ch )
+    }
+    else {
+        HISTOQC ( images, config_ch )
+    }


The error seems to suggest some behavioral difference between how config_ch is passed to HISTOQC on each side of this if/else statement.

adamjtaylor · 2024-02-07T17:41:36Z

There seems to be no tangible difference in how the images and CONVERT.out channels look: Both are a tuple of a meta map containing id and the image path.

For images this is the path to the original image`

[[id:CMU-1-Small-Region], /Users/ataylor/Documents/projects/htan/nf-histoqc/test_data/CMU-1-Small-Region.svs]

for CONVERT.out this is the path in the workdir where it was created

[[id:CMU-1-Small-Region], /Users/ataylor/Documents/projects/htan/nf-histoqc/work/1c/79d9b5d24e5cfd605822c62bcc0266/CMU-1-Small-Region_openslide.tiff]

adamjtaylor · 2024-02-07T17:44:51Z

From the command run there is also no tangible difference between if converted or not.

when --convert is set it is with a custom_config

nextflow run main.nf --input test_data/test_samplesheet.csv --mag 10 --mpp 0.1 --custom_config config_v2.1_pen.ini --profile local --convert

We have the following command executed

  echo "Using config: /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini"
  python -m histoqc -c /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini CMU-1-Small-Region-copy_openslide.tiff -o out
  mv out/CMU-1-Small-Region-copy_openslide.tiff/*.png .

When convert is not set we have

nextflow run main.nf --input test_data/test_samplesheet.csv --mag 10 --mpp 0.1 --custom_config config_v2.1_pen.ini --profile local

command.sh:

echo "Using config: /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini"
python -m histoqc -c /Users/ataylor/Documents/projects/htan/nf-histoqc/config_v2.1_pen.ini CMU-1-Small-Region-copy.svs -o out
mv out/CMU-1-Small-Region-copy.svs/*.png .

adamjtaylor · 2024-02-07T21:05:31Z

Also important to getting to the bottom of it is that the error is actually thrown by HistoQC itself. There is some internal logic (lines and python script mentioned in error) where it decides if the value passed to its -c argument is a path or the name of a built in config file.

adamjtaylor · 2024-02-07T22:18:15Z

Linked Jira ticket MC2DPQC-53

adamjtaylor · 2024-02-08T10:42:19Z

Here is where HistoQC attempts to read the config file

https://github.com/choosehappy/HistoQC/blob/fb9f9d64cd6ef22de84aa73c63055f5b84a02590/histoqc/__main__.py#L74-L82

    config = configparser.ConfigParser()
    if not args.config:
        lm.logger.warning(f"Configuration file not set (--config), using default")
        config.read_string(read_config_template('default'))
    elif os.path.exists(args.config):
        config.read(args.config) #Will read the config file
    else:
        lm.logger.warning(f"Configuration file {args.config} assuming to be a template...checking.")
        config.read_string(read_config_template(args.config))

adamjtaylor · 2024-02-08T10:45:05Z

So for some reason it seems to think that the path passed to args.config does not exist when we are passing the converted tiff file, but is fine with it when we pass the original svs file.

When it fails the os.path.exists elif statement it falls back to try and read the config as the name of an internal file with read_cofig_template which understandably then fails

def read_config_template(name=None):
    """return the contents of a configuration template"""
    templates = list_config_templates()
    if name not in templates:
        raise KeyError(f'no configuration template found under key {name!r}')
    return _resources.read_text('histoqc.config', templates[name])
    ```

adamjtaylor · 2024-02-09T10:08:53Z

I think that these fixes get us working.

config only

(base) ataylor@ajt-mbp nf-histoqc % nextflow run main.nf --input test_data/test_samplesheet.csv --mag 10 --mpp 0.1 --config first --profile local          
N E X T F L O W  ~  version 23.04.4
Launching `main.nf` [clever_lamport] DSL2 - revision: 3acc274e08
executor >  local (5)
[5d/821d92] process > NF_HISTOQC:RUN:HISTOQC (CMU-1-Small-Region-copy) [100%] 2 of 2 ✔
[4b/640375] process > NF_HISTOQC:COLLECT:RESULTS                       [100%] 1 of 1 ✔
[d7/04b246] process > NF_HISTOQC:COLLECT:TIDY                          [100%] 1 of 1 ✔
[16/149dd9] process > NF_HISTOQC:COLLECT:LOGS                          [100%] 1 of 1 ✔

config and convert:

(base) ataylor@ajt-mbp nf-histoqc % nextflow run main.nf --input test_data/test_samplesheet.csv --mag 10 --mpp 0.1 --config first --profile local --convert
N E X T F L O W  ~  version 23.04.4
Launching `main.nf` [prickly_lalande] DSL2 - revision: 3acc274e08
executor >  local (7)
[18/e0fda3] process > NF_HISTOQC:RUN:CONVERT (2)                       [100%] 2 of 2 ✔
[e4/b9c94b] process > NF_HISTOQC:RUN:HISTOQC (CMU-1-Small-Region-copy) [100%] 2 of 2 ✔
[3d/77b9c9] process > NF_HISTOQC:COLLECT:RESULTS                       [100%] 1 of 1 ✔
[26/1c618a] process > NF_HISTOQC:COLLECT:TIDY                          [100%] 1 of 1 ✔
[d6/f151e5] process > NF_HISTOQC:COLLECT:LOGS                          [100%] 1 of 1 ✔

custom config only

(base) ataylor@ajt-mbp nf-histoqc % nextflow run main.nf --input test_data/test_samplesheet.csv --mag 10 --mpp 0.1 --custom_config myconfig.ini --profile local          
N E X T F L O W  ~  version 23.04.4
Launching `main.nf` [loving_sax] DSL2 - revision: 3acc274e08
executor >  local (5)
[34/e0d666] process > NF_HISTOQC:RUN:HISTOQC (CMU-1-Small-Region-copy) [100%] 2 of 2 ✔
[5a/86c19f] process > NF_HISTOQC:COLLECT:RESULTS                       [100%] 1 of 1 ✔
[0a/70a1a0] process > NF_HISTOQC:COLLECT:TIDY                          [100%] 1 of 1 ✔
[de/69a626] process > NF_HISTOQC:COLLECT:LOGS                          [100%] 1 of 1 ✔

custom config and convert

(base) ataylor@ajt-mbp nf-histoqc % nextflow run main.nf --input test_data/test_samplesheet.csv --mag 10 --mpp 0.1 --custom_config myconfig.ini --profile local --convert
N E X T F L O W  ~  version 23.04.4
Launching `main.nf` [small_roentgen] DSL2 - revision: 3acc274e08
executor >  local (7)
[51/b7beec] process > NF_HISTOQC:RUN:CONVERT (2)                       [100%] 2 of 2 ✔
[f4/ff4410] process > NF_HISTOQC:RUN:HISTOQC (CMU-1-Small-Region-copy) [100%] 2 of 2 ✔
[db/05258c] process > NF_HISTOQC:COLLECT:RESULTS                       [100%] 1 of 1 ✔
[1c/187f5a] process > NF_HISTOQC:COLLECT:TIDY                          [100%] 1 of 1 ✔
[59/9b4b83] process > NF_HISTOQC:COLLECT:LOGS                          [100%] 1 of 1 ✔

BWMac · 2024-02-09T16:20:39Z

@adamjtaylor I was able to reproduce your successful runs on my local machine using the config file you provided in the Jira ticket. The changes look good!

adamjtaylor · 2024-02-09T18:43:06Z

Thanks @BWMac!

Remaining action on me is to update the README.

adamjtaylor added 4 commits February 7, 2024 12:53

Add method to use custom config file

bbf1462

Update schema

9ef7d18

Merge branch 'main' into allow-custom-config

ab29fa0

Reconfigure logic

6818def

tidy

c06a470

adamjtaylor commented Feb 7, 2024

View reviewed changes

adamjtaylor linked an issue Feb 8, 2024 that may be closed by this pull request

Add support for custom config files #21

Closed

adamjtaylor added 2 commits February 9, 2024 10:02

Fixes based on conversation with Brad

d904cd9

Cleanup

2253565

Update README

b9d4ce8

adamjtaylor merged commit dd3b395 into main Feb 11, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow use of a custom config file #29

Allow use of a custom config file #29

adamjtaylor commented Feb 7, 2024 •

edited

Loading

adamjtaylor commented Feb 7, 2024

adamjtaylor Feb 7, 2024 •

edited

Loading

adamjtaylor commented Feb 7, 2024

adamjtaylor commented Feb 7, 2024

adamjtaylor commented Feb 7, 2024

adamjtaylor commented Feb 7, 2024

adamjtaylor commented Feb 8, 2024

adamjtaylor commented Feb 8, 2024 •

edited

Loading

adamjtaylor commented Feb 9, 2024

BWMac commented Feb 9, 2024

adamjtaylor commented Feb 9, 2024

Allow use of a custom config file #29

Allow use of a custom config file #29

Conversation

adamjtaylor commented Feb 7, 2024 • edited Loading

adamjtaylor commented Feb 7, 2024

adamjtaylor Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

adamjtaylor commented Feb 7, 2024

adamjtaylor commented Feb 7, 2024

adamjtaylor commented Feb 7, 2024

adamjtaylor commented Feb 7, 2024

adamjtaylor commented Feb 8, 2024

adamjtaylor commented Feb 8, 2024 • edited Loading

adamjtaylor commented Feb 9, 2024

BWMac commented Feb 9, 2024

adamjtaylor commented Feb 9, 2024

adamjtaylor commented Feb 7, 2024 •

edited

Loading

adamjtaylor Feb 7, 2024 •

edited

Loading

adamjtaylor commented Feb 8, 2024 •

edited

Loading