AdamaJava / adamajava

Other
14 stars 5 forks source link

update picard version #282

Closed ChristinaXu2017 closed 3 years ago

ChristinaXu2017 commented 3 years ago

Description

Here we updated the Picard version, hence the new Picard is compatible with our current htsjdk. qpicard::QSamToFastq used Picard 1.130 which was dated and conflicts with our current htsjdk:2.14.1. There are no Picard jar calls htsjdk:2.14.1. But the Picard 2.17.7 calls htsjdk:2.14.0 and Picard 2.17.8 calls 2.14.2; We pickup Picard 2.17.7 here, and updated the importing library (org.broadinstitute.barclay.argparser.Argument) replacing picard.cmdline.Option.

The qpicard is a widely used project, but qpicard.fastq.QSamToFastq is an independent command-line tool, not integrated into any other adamajava tools. This qpicard does not require picard.jar library except this QSamToFastq. picard.jar is quite aggressive, it overrides some common java libraries, eg. OptionParserState.handleArgument(...). Here we move this independent tool from qpicard to qmule, hence our adamajava tools which use qpicard won't require to intall picard.jar.

Fixes#281

Type of change

How Has This Been Tested?

Update picard.jar version won't affect any other adamajava, so here we only test this command line tool.

I use the same command line, it threw exception but now it runs sucessfully. java -cp $ADAMA_HOME/build/lib/qpicard.jar org.qcmg.picard.fastq.QSamToFastq I=$bam FASTQ=$fr1 SECOND_END_FASTQ=$fr2 RC=Boolean

Are WDL Updates Required?

It is small tool, not involved in our current WDL pipeline

Checklist:

just change the importing library name and the build file. There is no code check requires

holmeso commented 3 years ago

Is there any reason you picked picard 2.10.0? Would it be a good idea to pick the latest picard release that was built for htsjdk 2.14.1? If so, I think that version 2.17.7 would be the one for you. Version 2.17.8 upgrades to htsjdk 2.14.2, so I presume the previous version is 2.14.1 compatible.

holmeso commented 3 years ago

It should also be noted that qpicard is a widely used project and that by updating the underlying picard jar, we may be introducing changes that will have an affect on our processes. Hopefully the regression tests will pick these up. That is not to say that we shouldn't update picard and htsjdk - we absolutely should. We just need to be aware of the consequences of such actions.

ChristinaXu2017 commented 3 years ago

It should also be noted that qpicard is a widely used project and that by updating the underlying picard jar, we may be introducing changes that will have an affect on our processes. Hopefully the regression tests will pick these up. That is not to say that we shouldn't update picard and htsjdk - we absolutely should. We just need to be aware of the consequences of such actions.

noted in test section of this pull request

ChristinaXu2017 commented 3 years ago

Is there any reason you picked picard 2.10.0? Would it be a good idea to pick the latest picard release that was built for htsjdk 2.14.1? If so, I think that version 2.17.7 would be the one for you. Version 2.17.8 upgrades to htsjdk 2.14.2, so I presume the previous version is 2.14.1 compatible.

Not sure whether the latest picard jar conflicts with our current htsjdk. I tried couple picard jars which released after our current htsjdk, the 2.1.0 is not compatible with our htsjdk, but 2.10.0 does.

We may update to the latest one only if we move htsjdk to the latest version. htsjdk is widely used in our in-house tools, it may heavily affect on our adamajava. We have to create another pull request If we decide to use the latest htsjdk.

holmeso commented 3 years ago

Is there any reason you picked picard 2.10.0? Would it be a good idea to pick the latest picard release that was built for htsjdk 2.14.1? If so, I think that version 2.17.7 would be the one for you. Version 2.17.8 upgrades to htsjdk 2.14.2, so I presume the previous version is 2.14.1 compatible.

Not sure whether the latest picard jar conflicts with our current htsjdk. I tried couple picard jars which released after our current htsjdk, the 2.1.0 is not compatible with our htsjdk, but 2.10.0 does.

We may update to the latest one only if we move htsjdk to the latest version. htsjdk is widely used in our in-house tools, it may heavily affect on our adamajava. We have to create another pull request If we decide to use the latest htsjdk.

I was suggesting that you use the latest version of picard that was written for the version of htsjdk that you are currently using

holmeso commented 3 years ago

Description updated to describe why class was moved from qpicard to qmule:

The qpicard is a widely used project, but qpicard.fastq.QSamToFastq is an independent command-line tool, not integrated into any other adamajava tools. This qpicard does not require picard.jar library except this QSamToFastq. picard.jar is quite aggressive, it overrides some common java libraries, eg. OptionParserState.handleArgument(...). Here we move this independent tool from qpicard to qmule, hence our adamajava tools which use qpicard won't require to intall picard.jar.

ChristinaXu2017 commented 3 years ago

thanks. yes, the description is updated.

holmeso commented 3 years ago

Looks to me like we need to rebrand qpicard to qhtsjdk so that there is no confusion as to what this project is about. For another day....