mc2-center / nf-histoqc

A Nextflow wrapper for HistoQC
MIT License
0 stars 1 forks source link

Allow use of a custom config file #29

Closed adamjtaylor closed 7 months ago

adamjtaylor commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

Linked Jira ticket MC2DPQC-53

adamjtaylor commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

Thanks @BWMac!

Remaining action on me is to update the README.