V3.0 #267
V3.0 #267
Conversation
Allows meridian to set its own word boundary flag since /a\.m\.\b/ does not match. The \b is now only included for forms without an abbreviation period.
Fixes nlp test case At '8PM on August 5th'
Fixes parsing of "August 25" with quotes.
…ormations Avoid discarding periods and quotes in nlp and parse
Allow day start hour to be configurable - by default 9 as now
Fixed code formatting for pycodestyle 2.2.0
Added simple logging in nlp
|
I don’t remember how much I finished up on that testing rewrite before my project using parsedatetime got shut down, but I’ll take a look. |
There was a problem hiding this comment.
Reviewable status: 0 of 13 files reviewed, 3 unresolved discussions (waiting on @bear and @idpaterson)
parsedatetime/init.py, line 244 at r5 (raw file):
VERSION_FLAG_STYLE = 1 VERSION_CONTEXT_STYLE = 2 DEFAULT_DAY_START_HOUR = 9
I am slightly concerned about redefining this here. If in version 2 someone had customized StartHour in Constants that change will no longer have any effect. Constants.StartHour still exists and still defaults to 9, so I do not believe that this separate constant is necessary.
parsedatetime/init.py, line 797 at r5 (raw file):
startSecond = sec else: startHour = self.day_start_hour
Ideally this would be something like startHour = self.ptc.StartHour if self.day_start_hour is None else self.day_start_hour to improve backwards compatibility.
tests/TestLocaleBase.py, line 132 at r5 (raw file):
super(TestDayOffsets, self).setUp() self.ptc = pdt.Constants('fr_FR', usePyICU=False) self.day_start_hour = 9
Is it necessary to set this to 9 in all of the tests? I occasionally like to look at unit tests for advanced code examples and this seems to be unnecessary. There is test coverage for the custom value so I don't think we need to pass in the default manually.
already being set in Constants In _evalModifier() initialize startHour from the current ptc object if current day_start_hour is NONE, otherwise pull it from the current day_start_hour
the tests that were testing that feature The Calendar class will now use the Constants member attribute value for StartHour as the default unless an explicit day_start_hour is passed
|
I have made the day_start_hour changes you requested |
There was a problem hiding this comment.
Sorry I didn't check back at the history on this one. So StartHour actually came from two separate pull requests, one in 2016 adding the day_start_hour param and another in 2019 adding Constants.StartHour. Do they serve distinct enough purposes to keep both? I'm not sure if it's contrived but I guess someone could have a customized Constants and create multiple Calendar instances with different start dates?
If that is the use case then assigning to self.ptc.StartHour would lead to unexpected behavior (subsequent calls with the shared Constants would start using the new start hour). I think it's less problematic to remove the day_start_hour param entirely in favor of letting the user manage Constants.StartHour. The param is a nice convenience, but it would need to be managed on the Calendar how it was originally implemented rather than changing ptc.StartHour
Reviewable status: 0 of 13 files reviewed, 3 unresolved discussions (waiting on @bear and @idpaterson)
working to get this merged with all of the main branch changes
This change is