check format of vague numbers. fixed yearsuffix equal to 0. fixed tru… #35
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ncate weeks for first days of the year
bethard
requested changes
Dec 21, 2017
| @@ -54,7 +54,8 @@ class AnaforaReader(val DCT: Interval)(implicit data: Data) { | |||
| } else if (value.forall(_.isDigit)) { | |||
| IntNumber(value.toInt) | |||
| } else { | |||
| VagueNumber(value) | |||
There was a problem hiding this comment.
I don't understand why this should be an error. Even if it's "2-3", isn't VagueNumber still an accurate description? If VagueNumber is giving you a problem in the scorer, maybe throw the exception in the scorer?
| @@ -619,8 +622,10 @@ private[formal] object RepeatingInterval { | |||
| case IsoFields.QUARTER_YEARS => | |||
| ldt.withMonth((ldt.getMonthValue - 1) / 4 + 1).withDayOfMonth(1).truncatedTo(ChronoUnit.DAYS) | |||
| case ChronoUnit.MONTHS => ldt.withDayOfMonth(1).truncatedTo(ChronoUnit.DAYS) | |||
| case ChronoUnit.WEEKS => | |||
| ldt.withDayOfYear(ldt.getDayOfYear - ldt.getDayOfWeek.getValue + 1).truncatedTo(ChronoUnit.DAYS) | |||
| case ChronoUnit.WEEKS => ldt.getDayOfYear - ldt.getDayOfWeek.getValue compare 0 match { | |||
There was a problem hiding this comment.
Seems like this could be something simpler like
ldt.withDayOfYear((ldt.getDayOfYear - ldt.getDayOfWeek.getValue + 1) max 1).truncatedTo(ChronoUnit.DAYS)
Would that achieve the same thing?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some numbers in the clincal data have the format "1-2" and produce match errors. I have added an exception when the number does not have a numeric format.
When a two digit year is equal to 00, YearSuffix produces a division by 0 error. I have tried to handle this.
If dayoftheyear - dayofthweek is less than 0 the truncation produces invalid values for dayofyear. If that happens I truncate to the day 1 of the year.