-
Notifications
You must be signed in to change notification settings - Fork 12
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
Scorer revised dev #33
Conversation
@@ -136,7 +136,10 @@ class AnaforaReader(val DCT: Interval)(implicit data: Data) { | |||
case ("Two-Digit-Year", Some(value), N, N, N, None) => value.partition(_ != '?') match { | |||
case (year, questionMarks) => YearSuffix(interval(properties), year.toInt, questionMarks.length) | |||
} | |||
case ("Between", None, N, N, N, None) => Between(interval(properties, "Start-"), interval(properties, "End-")) | |||
case ("Between", None, N, N, N, None) => interval(properties, "Start-").end == interval(properties, "End-").start match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should check the Start-Included
and End-Included
properties, not test whether the end and start are equal.
*/ | ||
case class Between(startInterval: Interval, endInterval: Interval) extends Interval { | ||
case class Between(startInterval: Interval, endInterval: Interval, inclusive: Boolean = false) extends Interval { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need separate startIncluded
and endIncluded
properties. Either one can be marked as included or excluded in the annotation.
@@ -15,6 +15,14 @@ object TimeNormScorer { | |||
case class Timex(id: String, textSpan: (Int, Int), time: TimeExpression) { | |||
val (textStart, textEnd) = textSpan | |||
|
|||
if (intervals(time).size == 0){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than doing this here, which makes it impossible to use Timex
for anything other than intervals, add a method Timex.allIntervalsFrom
that calls Timex.allFrom
and filters according to your rules here.
if (intervals(time).size == 0){ | ||
throw new IllegalStateException("Time expression without intervals") | ||
} | ||
for (interval <- intervals(time)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're trying to provoke an exception here, right? Can't you just check interval.isDefined
? That, coupled with the comment above, should allow you to simply filter, rather than throwing exceptions and using a Try
.
0eae479
to
29930ae
Compare
…luded and End-Included in Between type
No description provided.