Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thrax 11 Publish thrax to Sonatype #12

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

lewismc
Copy link

@lewismc lewismc commented May 14, 2016

This PR is an attempt at Mavenizing Thrax with the attempt of having an artifact which can be pushed to Maven Central.
Essentially this PR does exactly that however it also does not correctly compile due to API changes within the most recent version of [jerboa|https://github.com/vandurme/jerboa/]. In particular the Classes which are affected are

edu.jhu.thrax.hadoop.distributional.CommonLSH
edu.jhu.thrax.hadoop.distributional.ContextWritable
edu.jhu.thrax.hadoop.distributional.DistributionalContextReducer

@mjpost can you take a look at the APi changes and potentially implement them correctly as i am not sure what is going on.

@mjpost
Copy link
Member

mjpost commented Aug 15, 2016

@lewismc, we're pulling Thrax into Joshua as part of the multi-core move, so that will enable an easy Maven push (joshua-thrax). Sound okay to close this out?

@lewismc
Copy link
Author

lewismc commented Aug 15, 2016

Sure does @mjpost that would have been my preference. Thanks @mjpost

@lewismc lewismc closed this Aug 15, 2016
An attempt at setting this was in the code, but the constant was overriden with
a query to the config file.
@lewismc lewismc reopened this Oct 3, 2016
@lewismc
Copy link
Author

lewismc commented Oct 6, 2016

This build has been completely converted to Maven, slf4j-over-log4j has been implemented and all forbidden API's calls have been addressed however it is not ready to be merged into master yet as

  1. No one has fully tested it yet, and
  2. I get the following broken tests

Failed tests:   getLabel_NoConstCatCCG_returnDoubleCat(edu.jhu.thrax.extraction.SAMTLabelerTest): expected:<B+D+F> but was:<-11>
  getLabel_UnaryChain_Top(edu.jhu.thrax.extraction.SAMTLabelerTest): expected:<A> but was:<-2>
  getLabel_NoConst_returnCat(edu.jhu.thrax.extraction.SAMTLabelerTest): expected:<B+D> but was:<-10>
  getLabel_NoConstCat_returnCCG(edu.jhu.thrax.extraction.SAMTLabelerTest): expected:<A/D> but was:<-12>
  getLabel_UnaryChain_Bottom(edu.jhu.thrax.extraction.SAMTLabelerTest): expected:<B> but was:<-4>
  getLabel_UnaryChain_All(edu.jhu.thrax.extraction.SAMTLabelerTest): expected:<A:B> but was:<-13>
  numLeaves_Leaf_isOne(edu.jhu.thrax.syntax.ParseTreeTest): 0
  internalNodesWithSpan_Single(edu.jhu.thrax.syntax.ParseTreeTest): expected:<B> but was:<-2>
  internalNodesWithSpan_unaryChain(edu.jhu.thrax.syntax.ParseTreeTest): expected:<A> but was:<-1>
  leaf_ChildIterator_isEmpty(edu.jhu.thrax.syntax.ParseTreeTest): 0
  tree_ChildIterator(edu.jhu.thrax.syntax.ParseTreeTest): expected:<b> but was:<6>
  numNodes_Leaf_isOne(edu.jhu.thrax.syntax.ParseTreeTest): expected:<1> but was:<0>
  alignedSentencePair_simple(edu.jhu.thrax.util.io.InputUtilitiesTest): expected:<foo> but was:<1>
  getWords_PlainWords_ReturnsStringArray(edu.jhu.thrax.util.io.InputUtilitiesTest): expected:<[Ljava.lang.String;@60addb54> but was:<[I@3f2a3a5>
  parseYield_UnbalancedLeft_ThrowsException(edu.jhu.thrax.util.io.InputUtilitiesTest): (..)
  parseYield_UnbalancedRight_ThrowsException(edu.jhu.thrax.util.io.InputUtilitiesTest): (..)
  parseYield_Whitespace_ReturnsZeroLengthArray(edu.jhu.thrax.util.io.InputUtilitiesTest): malformed parse
  alignedSentencePair_reversed(edu.jhu.thrax.util.io.InputUtilitiesTest): expected:<bar> but was:<2>

Tests run: 47, Failures: 18, Errors: 0, Skipped: 0

@lewismc
Copy link
Author

lewismc commented Oct 6, 2016

@mjpost is there work going on elsewhere to Mavenize Thrax? I jsut re-read your message above... which may indicate that the work has been completed elsewhere. I hope to god that this is not the case and this PR has and is time well spent.

@lewismc
Copy link
Author

lewismc commented Oct 6, 2016

I'll wait for your reply here before continuing to investigate the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants