From ed05cf9893fc9f1163e7ec0541880d590bda0853 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Thu, 16 Feb 2012 10:05:07 +0000 Subject: [PATCH] Issue 320: Refactor stop_timezone validation code out of google extensions and into main validator Reviewed at http://codereview.appspot.com/5641063/ --- extensions/googletransit/stop.py | 18 +----------------- test/testgoogletransitextension.py | 10 ++-------- test/testtransitfeed.py | 23 +++++++++++++++++++++++ transitfeed/stop.py | 13 ++++++++++++- 4 files changed, 38 insertions(+), 26 deletions(-) diff --git a/extensions/googletransit/stop.py b/extensions/googletransit/stop.py index a1e9299f..103699a0 100644 --- a/extensions/googletransit/stop.py +++ b/extensions/googletransit/stop.py @@ -17,8 +17,6 @@ import transitfeed import transitfeed.util as util import transitfeed.problems as problems_module -import route as route_extension -import re class Stop(transitfeed.Stop): """Extension of transitfeed.Stop: @@ -28,16 +26,11 @@ class Stop(transitfeed.Stop): - Overriding ValidateStopLocationType(), adding location_type 2 (entrance). """ - _FIELD_NAMES = transitfeed.Stop._FIELD_NAMES + ['stop_timezone', - 'vehicle_type', + _FIELD_NAMES = transitfeed.Stop._FIELD_NAMES + ['vehicle_type', 'wheelchair_boarding'] LOCATION_TYPE_ENTRANCE = 2 - # New validation function for field 'stop_timezone'. - def ValidateStopTimezone(self, problems): - util.ValidateTimezone(self.stop_timezone, 'stop_timezone', problems) - # New validation function for field 'vehicle_type'. def ValidateVehicleType(self, problems): self.vehicle_type = util.ValidateAndReturnIntValue( @@ -63,7 +56,6 @@ def ValidateWheelchairBoarding(self, problems): # Overriding transitfeed.Stop.ValidateBeforeAdd(). def ValidateBeforeAdd(self, problems): super(Stop, self).ValidateBeforeAdd(problems) - self.ValidateStopTimezone(problems) self.ValidateVehicleType(problems) self.ValidateWheelchairBoarding(problems) return True # None of these checks are blocking @@ -77,14 +69,6 @@ def ValidateStopLocationType(self, problems): if self.location_type == 2 and util.IsEmpty(self.parent_station): problems.InvalidValue('location_type', self.location_type, reason='an entrance must have a parent_station') - # Entrances or other child stops (having a parent station) must not have a - # stop_timezone. - if (not util.IsEmpty(self.parent_station) and - not util.IsEmpty(self.stop_timezone)): - problems.InvalidValue('location_type', self.location_type, - reason='an entrance or a stop having a parent stop must not have a ' - 'stop_timezone', type=problems_module.TYPE_WARNING) - # Overriding _ReportMissingRequiredField() in order to allow empty stop_name # if location_type=2 (entrance). diff --git a/test/testgoogletransitextension.py b/test/testgoogletransitextension.py index 3f96bffb..318430ad 100644 --- a/test/testgoogletransitextension.py +++ b/test/testgoogletransitextension.py @@ -219,12 +219,6 @@ def setUp(self): self._entrance.parent_station = self._parent_stop.stop_id self._entrance._gtfs_factory = self.gtfs_factory - def testValidateStopTimezone(self): - self._stop.stop_timezone = 'TestTimeZone/Wrong_Place' - self._stop.Validate(self.problems) - e = self.accumulator.PopInvalidValue('stop_timezone') - self.accumulator.AssertNoMoreExceptions() - def testValidateVehicleType(self): # Test with non-integer value self._stop.vehicle_type = 'abc' @@ -246,7 +240,7 @@ def testEntranceExceptions(self): # An entrance must not have a stop_timezone self._entrance.stop_timezone = 'America/Los_Angeles' self._entrance.Validate(self.problems) - e = self.accumulator.PopInvalidValue('location_type') + e = self.accumulator.PopInvalidValue('stop_timezone') self.assertMatchesRegex(r'stop_timezone', e.FormatProblem()) self.accumulator.AssertNoMoreExceptions() self._entrance.stop_timezone = None @@ -273,7 +267,7 @@ def testChildExceptions(self): # A _child_stop must not have a stop_timezone self._child_stop.stop_timezone = 'America/Los_Angeles' self._child_stop.Validate(self.problems) - e = self.accumulator.PopInvalidValue('location_type') + e = self.accumulator.PopInvalidValue('stop_timezone') self.assertMatchesRegex(r'stop_timezone', e.FormatProblem()) self.assertTrue(e.IsWarning()) self.accumulator.AssertNoMoreExceptions() diff --git a/test/testtransitfeed.py b/test/testtransitfeed.py index 472491db..d4e88634 100644 --- a/test/testtransitfeed.py +++ b/test/testtransitfeed.py @@ -1119,6 +1119,12 @@ def runTest(self): self.ValidateAndExpectInvalidValue(stop, 'stop_desc') stop.stop_desc = 'Edge of the Couch' self.accumulator.AssertNoMoreExceptions() + + stop.stop_timezone = 'This_Timezone/Does_Not_Exist' + self.ValidateAndExpectInvalidValue(stop, 'stop_timezone') + stop.stop_timezone = 'America/Los_Angeles' + stop.Validate(self.problems) + self.accumulator.AssertNoMoreExceptions() class StopAttributes(ValidationTestCase): @@ -1930,6 +1936,23 @@ def testStopTooFarFromParentStation(self): " station Bullfrog (ID BULLFROG_ST)") != -1) self.accumulator.AssertNoMoreExceptions() + def testStopTimeZone(self): + self.SetArchiveContents( + "stops.txt", + "stop_id,stop_name,stop_lat,stop_lon,location_type,parent_station," + "stop_timezone\n" + "BEATTY_AIRPORT,Airport,36.868446,-116.784582,,STATION," + "America/New_York\n" + "STATION,Airport,36.868446,-116.784582,1,,\n" + "BULLFROG,Bullfrog,36.88108,-116.81797,,,\n" + "STAGECOACH,Stagecoach Hotel,36.915682,-116.751677,,,\n") + self.MakeLoaderAndLoad() + e = self.accumulator.PopException("InvalidValue") + self.assertEqual(1, e.type) # Warning + self.assertEquals(2, e.row_num) + self.assertEquals("stop_timezone", e.column_name) + self.accumulator.AssertNoMoreExceptions() + #Uncomment once validation is implemented #def testStationWithoutReference(self): # self.SetArchiveContents( diff --git a/transitfeed/stop.py b/transitfeed/stop.py index 33cd61e1..0f579731 100755 --- a/transitfeed/stop.py +++ b/transitfeed/stop.py @@ -38,7 +38,7 @@ class Stop(GtfsObjectBase): _REQUIRED_FIELD_NAMES = ['stop_id', 'stop_name', 'stop_lat', 'stop_lon'] _FIELD_NAMES = _REQUIRED_FIELD_NAMES + \ ['stop_desc', 'zone_id', 'stop_url', 'stop_code', - 'location_type', 'parent_station'] + 'location_type', 'parent_station', 'stop_timezone'] _TABLE_NAME = 'stops' LOCATION_TYPE_STATION = 1 @@ -221,6 +221,16 @@ def ValidateStopIsNotStationWithParent(self, problems): 'Stop row with location_type=1 (a station) must ' 'not have a parent_station') + def ValidateStopTimezone(self, problems): + # Entrances or other child stops (having a parent station) must not have a + # stop_timezone. + util.ValidateTimezone(self.stop_timezone, 'stop_timezone', problems) + if (not util.IsEmpty(self.parent_station) and + not util.IsEmpty(self.stop_timezone)): + problems.InvalidValue('stop_timezone', self.stop_timezone, + reason='a stop having a parent stop must not have a stop_timezone', + type=problems_module.TYPE_WARNING) + def ValidateBeforeAdd(self, problems): # First check that all required fields are present because ParseAttributes # may remove invalid attributes. @@ -233,6 +243,7 @@ def ValidateBeforeAdd(self, problems): self.ValidateStopLongitude(problems) self.ValidateStopUrl(problems) self.ValidateStopLocationType(problems) + self.ValidateStopTimezone(problems) # Check that this object is consistent with itself self.ValidateStopNotTooCloseToOrigin(problems)