hammerlab / epidisco

Personalized cancer epitope discovery and peptide vaccine prediction pipeline
Apache License 2.0
30 stars 7 forks source link

`name` is now a named argument to the `save` method #187

Closed armish closed 7 years ago

armish commented 7 years ago

so that we can compile epidisco once again.

ihodes commented 7 years ago

Sorry for the slow review, missed this PR. I think we should stick with using the Epidisco save instead of the new one form Biokepi, as we get some extra goodies with the one in Epidisco that we don't with Biokepi (like json blobs).

armish commented 7 years ago

My bad - should have explained this PR better:

The problem is that since the addition of the save (https://github.com/hammerlab/biokepi/pull/452) to the biokepi, epidisco doesn't compile and complain about the signature mismatch between the biokepi and the epidisco one:

File "src/lib/extended_edsl.ml", line 1:
Error: The files src/lib/save_result.cmi
       and /Users/arman/.opam/4.03.0/lib/biokepi/biokepi.cmi
       make inconsistent assumptions over interface Biokepi
Command exited with code 2.
Compilation unsuccessful after building 8 targets (7 cached) in 00:00:00.
make: *** [byte] Error 10

So this PR is not meant to use the biokepi one but instead just makes its signature match to be able to compile epidisco.

Let me know if I am missing something. No rush here, though. I am already working with a local mix-and-match version of biokepi/epidisco, this is not a blocker for me at this moment.

Details and sanity checks:

arman@narnia epidisco > gcom
Already on 'master'
Your branch is up-to-date with 'origin/master'.
arman@narnia epidisco > gln 1

HEAD -> master origin/master origin/HEAD netmhc-sync b414585 3wk Sebastien Mondet 2017-05-04 16:44:38 Merge PR #186 (@smondet, switch to `solvuu-build`) 

arman@narnia epidisco > git pull
Already up-to-date.
arman@narnia epidisco > gs
On branch master
Your branch is up-to-date with 'origin/master'.
nothing to commit, working directory clean
arman@narnia epidisco > make
ocamlbuild -use-ocamlfind -plugin-tag "package(solvuu-build,nonstd)" .merlin
Finished, 0 targets (0 cached) in 00:00:00.
Finished, 1 target (1 cached) in 00:00:00.
rm -f .merlin
ln -fs _build/.merlin .merlin
ocamlbuild -use-ocamlfind -plugin-tag "package(solvuu-build,nonstd)" .ocamlinit
Finished, 0 targets (0 cached) in 00:00:00.
Finished, 1 target (1 cached) in 00:00:00.
rm -f .ocamlinit
ln -fs _build/.ocamlinit .ocamlinit
ocamlbuild -use-ocamlfind -plugin-tag "package(solvuu-build,nonstd)" src/epidisco.cma
Finished, 0 targets (0 cached) in 00:00:00.
fatal: No names found, cannot describe anything.
+ ocamlfind ocamlc -bin-annot -c -for-pack Epidisco -I src/lib -o src/lib/extended_edsl.cmo -thread -package biokepi,ppx_deriving.std,ppx_deriving_cmdliner src/lib/extended_edsl.ml
File "src/lib/extended_edsl.ml", line 1:
Error: The files src/lib/save_result.cmi
       and /Users/arman/.opam/4.03.0/lib/biokepi/biokepi.cmi
       make inconsistent assumptions over interface Biokepi
Command exited with code 2.
Compilation unsuccessful after building 8 targets (7 cached) in 00:00:00.
make: *** [byte] Error 10
arman@narnia epidisco > opam info biokepi
             package: biokepi
             version: ~unknown
              pinned: git (44068d01)
        upstream-url: /Users/arman/Projects/biokepi
       upstream-kind: git
              author: Sebastien Mondet <seb@mondet.org>, Leonid Rozenberg <leonidr@gmail.com>, Arun Ahuja <aahuja11@gmail.com>, Jeff Hammerbacher <jeff.hammerbacher@gmail.com>, Isaac Hodes <isaachodes@gmail.com>, Bulent Arman Aksoy <arman@aksoy.org>
             license: Apache-2.0
             depends: ocamlbuild & solvuu-build >= 0.3.0 & ocamlfind & base-threads & ketrew >= 2.0.0
   installed-version: ~unknown [4.03.0]
   available-version: ~unknown
         description: Bioinformatics Ketrew Pipelines
ihodes commented 7 years ago

Interesting, naïvely I'd expect the Epidisco save to overwrite the Biokepi one, and for this to compile without an issue…!

LGTM, thanks for fixing!