bump v1.1.0

[nservant]

Many thanks to contributing to nf-core/hic!

Please fill in the appropriate checklist below (delete whatever is not relevant). These are the most common things requested on pull requests (PRs).

PR checklist

• This comment contains a description of changes (with reason)
• If you've fixed a bug or added code that should be tested, add tests!
• If necessary, also make a PR on the nf-core/hic branch on the nf-core/test-datasets repo
• Ensure the test suite passes (nextflow run . -profile test,docker).
• Make sure your code lints (nf-core lint .).
• Documentation in docs is updated
• CHANGELOG.md is updated
• README.md is updated

Learn more about contributing: https://github.com/nf-core/hic/tree/master/.github/CONTRIBUTING.md

[apeltzer]

Will wait until Travis finishes for review

[nservant]

hummm ... still this 'lint' issue I guess ...

[nservant]

conda search -c bioconda pysam | grep 0.15.2

is working well ... why lint cannot find it ?

[nservant]

Fixed by moving to lint 1.7
New error on CI
The command "wget -qO- get.nextflow.io | bash" failed and exited with 1 during .

[nservant]

ready for review !

[apeltzer]

Some minor stuff - mainly the docs are to be updated to link to nf-core webpage documentation for the central portions (see review in detail or ask 👍 )

Otherwise looking really good, thanks a bunch!

[nservant]

ok. Docs have been updated and replaced by links to the main nf-core.
Useless md files have been removed

[nservant]

Thanks for the review Alex ! Should we wait for another review ? or I can force the merge ?

[apeltzer]

Two reviews for everything on master please 👍 I'll link and ask @nf-core/core for a review :-)

[drpatelh]

Looks good @nservant . It seems like you have "partly" updated the code to reflect tools v1.7? Possibly worth updating all of the files because it will make the merge easier when the automated syncing comes into play.

Few questions/amendments:

• It seems you have updated the main README to point to the docs on the website but you will also need to update:
  https://github.com/nf-core/hic/blob/dev/docs/README.md
• conf/curie.config should be uploaded to nf-core/configs and so can be removed here. Not entirely sure what hicpro.config is doing?
• Update base.config to use process label instead of withName:
  https://github.com/nf-core/tools/blob/2d324b4334658e1cd907289b4a7497abf5ef4191/nf_core/pipeline-template/%7B%7Bcookiecutter.name_noslash%7D%7D/conf/base.config#L29-L46
• The indentation in process sections dont seem to be consistent.
• def nfcoreHeader() in main.nf has been fixed in v1.7 template to remove the dreaded 2m] print to screen. You may want to update this.
• multiqc_config.yaml is present in conf/ and assets/. Keep the one in the latter.
• Only some parameters are printed here:
  

  hic/main.nf

  Line 240 in 97d3d5d

  summary['splitFastq'] = params.splitFastq

• Step numbering isnt correct:
  

  hic/main.nf

  Line 835 in 97d3d5d

  * STEP 3 - Output Description HTML

• You could re-order/segment the parameter listing a little better with sub-headings because its a bit confusing at the moment as to which part of the pipeline that parameters will be used:
  

  hic/main.nf

  Lines 38 to 61 in 97d3d5d

  	Options:
  	--bwt2_opts_end2end Options for bowtie2 end-to-end mappinf (first mapping step). See hic.config for default.
  	--bwt2_opts_trimmed Options for bowtie2 mapping after ligation site trimming. See hic.config for default.
  	--min_mapq Minimum mapping quality values to consider. Default: 10
  	--restriction_site Cutting motif(s) of restriction enzyme(s) (comma separated). Default: 'A^AGCTT'
  	--ligation_site Ligation motifs to trim (comma separated). Default: 'AAGCTAGCTT'
  	--min_restriction_fragment_size Minimum size of restriction fragments to consider. Default: None
  	--max_restriction_framgnet_size Maximum size of restriction fragmants to consider. Default: None
  	--min_insert_size Minimum insert size of mapped reads to consider. Default: None
  	--max_insert_size Maximum insert size of mapped reads to consider. Default: None
  	--saveInteractionBAM Save BAM file with interaction tags (dangling-end, self-circle, etc.). Default: False
  	--dnase Run DNase Hi-C mode. All options related to restriction fragments are not considered. Default: False
  	--min_cis_dist Minimum intra-chromosomal distance to consider. Default: None
  	--rm_singleton Remove singleton reads. Default: true
  	--rm_multi Remove multi-mapped reads. Default: true
  	--rm_dup Remove duplicates. Default: true
  	--bin_size Bin size for contact maps (comma separated). Default: '1000000,500000'
  	--ice_max_iter Maximum number of iteration for ICE normalization. Default: 100
  	--ice_filter_low_count_perc Percentage of low counts columns/rows to filter before ICE normalization. Default: 0.02
  	--ice_filter_high_count_perc Percentage of high counts columns/rows to filter before ICE normalization. Default: 0
  	--ice_eps Convergence criteria for ICE normalization. Default: 0.1

  
  e.g.
  https://github.com/nf-core/chipseq/blob/333bba334512625775157aa0087c023cf0bde397/main.nf#L28-L72

[nservant]

Fixed:

• It seems you have updated the main README to point to the docs on the website but you will also need to update: https://github.com/nf-core/hic/blob/dev/docs/README.md
• conf/curie.config should be uploaded to nf-core/configs and so can be removed here. Not entirely sure what hicpro.config is doing?
• def nfcoreHeader() in main.nf has been fixed in v1.7 template to remove the dreaded 2m] print to screen. You may want to update this.
• multiqc_config.yaml is present in conf/ and assets/. Keep the one in the latter.
• Step numbering isnt correct:
• You could re-order/segment the parameter listing a little better with sub-headings because its a bit confusing at the moment as to which part of the pipeline that parameters will be used

Requires more discussion:

• Update base.config to use process label instead of withName

Why ? I like the idea of specifing for each tool the best parameters ... I'm not a big fan of 'low', 'medium', 'high' groups of process ...

• The indentation in process sections dont seem to be consistent.

Could give me some lines please ?

• Only some parameters are printed here:

Not sure to understand what you mean
Thanks

[drpatelh]

For some processes you have indented the lines after the input and output (personally, I find this quite difficult to read):

hic/main.nf

Lines 407 to 413 in 3273c92

	input:
	set val(sample), file(reads) from raw_reads
	file index from bwt2_index_end2end.collect()
	output:
	set val(prefix), file("${prefix}_unmap.fastq") into unmapped_end_to_end
	set val(prefix), file("${prefix}.bam") into end_to_end_bam

and for others you havent:

hic/main.nf

Lines 381 to 385 in 3273c92

	input:
	file fasta from fasta_for_resfrag
	output:
	file "*.bed" into res_frag_file

[drpatelh]

You have many more parameters in the pipeline that arent specified here and therefore they wont be printed at run-time:

hic/main.nf

Lines 241 to 248 in 3273c92

	summary['Reads'] = params.reads
	summary['splitFastq'] = params.splitFastq
	summary['Fasta Ref'] = params.fasta
	summary['Restriction Motif']= params.restriction_site
	summary['Ligation Motif'] = params.ligation_site
	summary['DNase Mode'] = params.dnase
	summary['Remove Dup'] = params.rm_dup
	summary['Maps resolution'] = params.bin_size

[drpatelh]

If you want to keep the resource definitions in base.config as they are then its fine 👍 They are in place mainly to simplify base.config especially for the larger pipelines. I mainly asked for standardisation purposes. Ideally, they are specified per process (as you have) but in most instances its difficult to know exactly what these are without extensive bench marking on a range of input data.

[nservant]

@drpatelh everything is fixed now, except i/ pictures in the doc and ii/ options format
I open new issues for that and will try to fix them in the next release if it's fine with you !?
Thanks

[drpatelh]

Ok. Thanks @nservant . Ill have another look.

[drpatelh]

@nservant . Looks much better. Thanks for making those changes. Couple more minor things I just noticed.

• "message" and not "emssage" here:
  

  hic/main.nf

  Line 88 in f9768e9

  // Show help emssage

• Need to change this line to get rid of the 2m] too:
  

  hic/main.nf

  Line 284 in f9768e9

  log.info "\033[2m----------------------------------------------------\033[0m"

  
  see:
  https://github.com/nf-core/tools/blob/2d324b4334658e1cd907289b4a7497abf5ef4191/nf_core/pipeline-template/%7B%7Bcookiecutter.name_noslash%7D%7D/main.nf#L149
• Remove Fmt in this code block. See:
  https://github.com/nf-core/tools/blob/2d324b4334658e1cd907289b4a7497abf5ef4191/nf_core/pipeline-template/%7B%7Bcookiecutter.name_noslash%7D%7D/main.nf#L363-L367

As above, there are a few other things too that would have been sorted out with a full sync of the latest pipeline template. I guess they can wait until the next release.

Happy for you to merge after you have sorted out the listed comments 👍

Also, you will need to add a date to the CHANGELOG when you merge to master

[nservant]

ok, last changes done ! Thanks @drpatelh