Issue 1493041: created new class for Pattern and shape desc
DescriptionCreat new classes for Pattern and relationship between pattern and trip, which is the preparation for the direct query from database instead of memory objects later.
Patch Set 1 #Patch Set 2 : updated #
Total comments: 6
Patch Set 3 : updated #Patch Set 4 : updated #
Total comments: 2
Patch Set 5 : updated #Patch Set 6 : updated #Patch Set 7 : updated #
Total comments: 4
Patch Set 8 : updated #
Total comments: 2
Patch Set 9 : updated #MessagesTotal messages: 25
1.added Pattern and ShapeDesc database objects 2.store relationships into database now
Sign in to reply to this message.
will someone please review this cl? 2010/6/3 <calidion@gmail.com> > Reviewers: lychen, xan_google.com, weiliu, leio.chen, baiming, > > Message: > 1.added Pattern and ShapeDesc database objects > 2.store relationships into database now > > > > Please review this at http://codereview.appspot.com/1493041/show > > Affected files: > M transitfeed_editor.py > > > Index: transitfeed_editor.py > =================================================================== > --- transitfeed_editor.py (revision 84) > +++ transitfeed_editor.py (working copy) > @@ -1085,7 +1085,16 @@ > > def _BindTripShapeStopTimesInfo(self): > """initialize relationship between entities""" > - > + if '_imported' not in dir(self) or not self._imported: > + self._db.CreateTable(Pattern._FIELD_NAMES, > + Pattern._FIELD_TYPES, > + Pattern._TABLE_NAME) > + self._db.CreateTable(ShapeDesc._FIELD_NAMES, > + ShapeDesc._FIELD_TYPES, > + ShapeDesc._TABLE_NAME) > + rows = [] > + descs = {} > + shape_desc_rows = [] > # Self.GetStopTimesOfAll() returns an array in format of > # (trip_id, stop_id, stop_sequence, departure_time, arrival_time) > stoptimes_of_all = self._db.GetStopTimesOfAll() > @@ -1155,6 +1164,15 @@ > len(full_stop_info_list)) > trip.start_time = stoptimes_in_trip[relation[0]][0][3] > > + if '_imported' not in dir(self) or not self._imported: > + rows.append([trip.pattern_id, trip.trip_id, trip.start_time]) > + if trip.shape_id not in descs: > + descs[trip.shape_id] = True > + shape_desc_rows.append([trip.shape_id, trip.shape_desc]) > + if '_imported' not in dir(self) or not self._imported: > + self._db.InsertRows(rows, Pattern._FIELD_NAMES, Pattern._TABLE_NAME) > + self._db.InsertRows(shape_desc_rows, ShapeDesc._FIELD_NAMES, > ShapeDesc._TABLE_NAME) > + > def _LoadShapesIntoObjects(self, shape_class): > header = shape_class._FIELD_NAMES > rows = self._db.GetAll(shape_class._TABLE_NAME) > @@ -1226,3 +1244,12 @@ > > return self._schedule > > +class Pattern(): > + _TABLE_NAME = 'patterns' > + _FIELD_NAMES = ['pattern_id', 'trip_id', 'start_time'] > + _FIELD_TYPES = ['char(256)', 'char(256)', 'char(256)'] > + > +class ShapeDesc(): > + _TABLE_NAME = 'shapedescs' > + _FIELD_NAMES = ['shape_id', 'shape_desc'] > + _FIELD_TYPES = ['char(256)', 'char(256)'] > > >
Sign in to reply to this message.
Sorry, looking into it now. On Thu, Jun 3, 2010 at 5:32 PM, <李白> wrote: > will someone please review this cl? > > 2010/6/3 <calidion@gmail.com> > > Reviewers: lychen, xan_google.com, weiliu, leio.chen, baiming, >> >> Message: >> 1.added Pattern and ShapeDesc database objects >> 2.store relationships into database now >> >> >> >> Please review this at http://codereview.appspot.com/1493041/show >> >> Affected files: >> M transitfeed_editor.py >> >> >> Index: transitfeed_editor.py >> =================================================================== >> --- transitfeed_editor.py (revision 84) >> +++ transitfeed_editor.py (working copy) >> @@ -1085,7 +1085,16 @@ >> >> def _BindTripShapeStopTimesInfo(self): >> """initialize relationship between entities""" >> - >> + if '_imported' not in dir(self) or not self._imported: >> + self._db.CreateTable(Pattern._FIELD_NAMES, >> + Pattern._FIELD_TYPES, >> + Pattern._TABLE_NAME) >> + self._db.CreateTable(ShapeDesc._FIELD_NAMES, >> + ShapeDesc._FIELD_TYPES, >> + ShapeDesc._TABLE_NAME) >> + rows = [] >> + descs = {} >> + shape_desc_rows = [] >> # Self.GetStopTimesOfAll() returns an array in format of >> # (trip_id, stop_id, stop_sequence, departure_time, arrival_time) >> stoptimes_of_all = self._db.GetStopTimesOfAll() >> @@ -1155,6 +1164,15 @@ >> len(full_stop_info_list)) >> trip.start_time = stoptimes_in_trip[relation[0]][0][3] >> >> + if '_imported' not in dir(self) or not self._imported: >> + rows.append([trip.pattern_id, trip.trip_id, trip.start_time]) >> + if trip.shape_id not in descs: >> + descs[trip.shape_id] = True >> + shape_desc_rows.append([trip.shape_id, trip.shape_desc]) >> + if '_imported' not in dir(self) or not self._imported: >> + self._db.InsertRows(rows, Pattern._FIELD_NAMES, >> Pattern._TABLE_NAME) >> + self._db.InsertRows(shape_desc_rows, ShapeDesc._FIELD_NAMES, >> ShapeDesc._TABLE_NAME) >> + >> def _LoadShapesIntoObjects(self, shape_class): >> header = shape_class._FIELD_NAMES >> rows = self._db.GetAll(shape_class._TABLE_NAME) >> @@ -1226,3 +1244,12 @@ >> >> return self._schedule >> >> +class Pattern(): >> + _TABLE_NAME = 'patterns' >> + _FIELD_NAMES = ['pattern_id', 'trip_id', 'start_time'] >> + _FIELD_TYPES = ['char(256)', 'char(256)', 'char(256)'] >> + >> +class ShapeDesc(): >> + _TABLE_NAME = 'shapedescs' >> + _FIELD_NAMES = ['shape_id', 'shape_desc'] >> + _FIELD_TYPES = ['char(256)', 'char(256)'] >> >> >> > -- Best Regards, Wei Liu 86-10-62503256(o)
Sign in to reply to this message.
What is your change designed for? http://codereview.appspot.com/1493041/diff/3001/2002 File transitfeed_editor.py (right): http://codereview.appspot.com/1493041/diff/3001/2002#newcode1168 transitfeed_editor.py:1168: shape_desc_rows.append([trip.pattern_id, trip.shape_id, trip.shape_desc]) Fix line length http://codereview.appspot.com/1493041/diff/3001/2002#newcode1171 transitfeed_editor.py:1171: self._db.InsertRows(shape_desc_rows, ShapeDesc._FIELD_NAMES, ShapeDesc._TABLE_NAME) Fix line length http://codereview.appspot.com/1493041/diff/3001/2002#newcode1251 transitfeed_editor.py:1251: _FIELD_NAMES = ['pattern_id', 'shape_id', 'shape_desc'] What does the field "shape_desc" mean? Seems no corresponding field in GTFS.
Sign in to reply to this message.
updated http://codereview.appspot.com/1493041/diff/3001/2002 File transitfeed_editor.py (right): http://codereview.appspot.com/1493041/diff/3001/2002#newcode1168 transitfeed_editor.py:1168: shape_desc_rows.append([trip.pattern_id, trip.shape_id, trip.shape_desc]) On 2010/06/03 09:42:50, weiliu wrote: > Fix line length Done. http://codereview.appspot.com/1493041/diff/3001/2002#newcode1171 transitfeed_editor.py:1171: self._db.InsertRows(shape_desc_rows, ShapeDesc._FIELD_NAMES, ShapeDesc._TABLE_NAME) On 2010/06/03 09:42:50, weiliu wrote: > Fix line length Done. http://codereview.appspot.com/1493041/diff/3001/2002#newcode1251 transitfeed_editor.py:1251: _FIELD_NAMES = ['pattern_id', 'shape_id', 'shape_desc'] On 2010/06/03 09:42:50, weiliu wrote: > What does the field "shape_desc" mean? Seems no corresponding field in GTFS. yes, it is for the sake of the consistence with the schedule viewer, the viewer has the description for each pattern
Sign in to reply to this message.
I still don't understand what's the purpose of this change. Is it to prepare sth? On Thu, Jun 3, 2010 at 5:52 PM, <calidion@gmail.com> wrote: > updated > > > > http://codereview.appspot.com/1493041/diff/3001/2002 > File transitfeed_editor.py (right): > > http://codereview.appspot.com/1493041/diff/3001/2002#newcode1168 > transitfeed_editor.py:1168: shape_desc_rows.append([trip.pattern_id, > trip.shape_id, trip.shape_desc]) > On 2010/06/03 09:42:50, weiliu wrote: > >> Fix line length >> > > Done. > > > http://codereview.appspot.com/1493041/diff/3001/2002#newcode1171 > transitfeed_editor.py:1171: self._db.InsertRows(shape_desc_rows, > ShapeDesc._FIELD_NAMES, ShapeDesc._TABLE_NAME) > On 2010/06/03 09:42:50, weiliu wrote: > >> Fix line length >> > > Done. > > > http://codereview.appspot.com/1493041/diff/3001/2002#newcode1251 > transitfeed_editor.py:1251: _FIELD_NAMES = ['pattern_id', 'shape_id', > 'shape_desc'] > On 2010/06/03 09:42:50, weiliu wrote: > >> What does the field "shape_desc" mean? Seems no corresponding field in >> > GTFS. > > yes, it is for the sake of the consistence with the schedule viewer, the > viewer has the description for each pattern Can you give me an example of shape_desc? Does user need to write some description for each shape? If you want to be consistent with schedule viewer, why not use pattern_desc? > > > http://codereview.appspot.com/1493041/show > -- Best Regards, Wei Liu 86-10-62503256(o)
Sign in to reply to this message.
yes. it is the database preparation for request changes later. 2010/6/3 Wei Liu <weiliu@google.com> > I still don't understand what's the purpose of this change. Is it to > prepare sth? > > On Thu, Jun 3, 2010 at 5:52 PM, <calidion@gmail.com> wrote: > >> updated >> >> >> >> http://codereview.appspot.com/1493041/diff/3001/2002 >> File transitfeed_editor.py (right): >> >> http://codereview.appspot.com/1493041/diff/3001/2002#newcode1168 >> transitfeed_editor.py:1168: shape_desc_rows.append([trip.pattern_id, >> trip.shape_id, trip.shape_desc]) >> On 2010/06/03 09:42:50, weiliu wrote: >> >>> Fix line length >>> >> >> Done. >> >> >> http://codereview.appspot.com/1493041/diff/3001/2002#newcode1171 >> transitfeed_editor.py:1171: self._db.InsertRows(shape_desc_rows, >> ShapeDesc._FIELD_NAMES, ShapeDesc._TABLE_NAME) >> On 2010/06/03 09:42:50, weiliu wrote: >> >>> Fix line length >>> >> >> Done. >> >> >> http://codereview.appspot.com/1493041/diff/3001/2002#newcode1251 >> transitfeed_editor.py:1251: _FIELD_NAMES = ['pattern_id', 'shape_id', >> 'shape_desc'] >> On 2010/06/03 09:42:50, weiliu wrote: >> >>> What does the field "shape_desc" mean? Seems no corresponding field in >>> >> GTFS. >> >> yes, it is for the sake of the consistence with the schedule viewer, the >> viewer has the description for each pattern > > > Can you give me an example of shape_desc? Does user need to write some > description for each shape? > > If you want to be consistent with schedule viewer, why not use > pattern_desc? > >> >> >> http://codereview.appspot.com/1493041/show >> > > > > -- > Best Regards, > Wei Liu > 86-10-62503256(o) >
Sign in to reply to this message.
On Thu, Jun 3, 2010 at 6:04 PM, <李白> wrote: > yes. it is the database preparation for request changes later. > Please add it to your changelist description like: "Creat new classes for Pattern and shape desc, which is the database preparation for ..." > 2010/6/3 Wei Liu <weiliu@google.com> > >> http://codereview.appspot.com/1493041/diff/3001/2002#newcode1251 >>> transitfeed_editor.py:1251: _FIELD_NAMES = ['pattern_id', 'shape_id', >>> 'shape_desc'] >>> On 2010/06/03 09:42:50, weiliu wrote: >>> >>>> What does the field "shape_desc" mean? Seems no corresponding field in >>>> >>> GTFS. >>> >>> yes, it is for the sake of the consistence with the schedule viewer, the >>> viewer has the description for each pattern >> >> >> Can you give me an example of shape_desc? Does user need to write some >> description for each shape? >> >> If you want to be consistent with schedule viewer, why not use >> pattern_desc? >> > Answer for this question? -- Best Regards, Wei Liu 86-10-62503256(o)
Sign in to reply to this message.
updated. 1.change Pattern to PatternTrip, which describe for the relationship between trip and pattern 2.change ShapeDesc to Pattern, which describes the relationship between the pattern and the route, and the desc for each pattern of the trips in the route.
Sign in to reply to this message.
2010/6/3 Wei Liu <weiliu@google.com> > > > On Thu, Jun 3, 2010 at 6:04 PM, <李白> wrote: > >> yes. it is the database preparation for request changes later. >> > > Please add it to your changelist description like: > "Creat new classes for Pattern and shape desc, which is the database > preparation for ..." > > >> 2010/6/3 Wei Liu <weiliu@google.com> >> >>> http://codereview.appspot.com/1493041/diff/3001/2002#newcode1251 >>>> >>>> transitfeed_editor.py:1251: _FIELD_NAMES = ['pattern_id', 'shape_id', >>>> 'shape_desc'] >>>> On 2010/06/03 09:42:50, weiliu wrote: >>>> >>>>> What does the field "shape_desc" mean? Seems no corresponding field in >>>>> >>>> GTFS. >>>> >>>> yes, it is for the sake of the consistence with the schedule viewer, the >>>> viewer has the description for each pattern >>>> >>> >>> Can you give me an example of shape_desc? Does user need to write some >>> description for each shape? >>> >>> If you want to be consistent with schedule viewer, why not use >>> pattern_desc? >>> >> > Answer for this question? > > changed shape_desc to pattern_desc > > > -- > Best Regards, > Wei Liu > 86-10-62503256(o) >
Sign in to reply to this message.
Please update cl description as I mentioned before. http://codereview.appspot.com/1493041/diff/14001/15001 File transitfeed_editor.py (right): http://codereview.appspot.com/1493041/diff/14001/15001#newcode1172 transitfeed_editor.py:1172: patterns[trip.pattern_id] = True If you are just checking whether a pattern_id is in patterns, why not initialize patterns as []?
Sign in to reply to this message.
On 2010/06/04 01:57:41, weiliu wrote: > Please update cl description as I mentioned before. Sorry I saw the update of your cl description, but what does "direct query" mean? Please make it clearer. Thanks! > > http://codereview.appspot.com/1493041/diff/14001/15001 > File transitfeed_editor.py (right): > > http://codereview.appspot.com/1493041/diff/14001/15001#newcode1172 > transitfeed_editor.py:1172: patterns[trip.pattern_id] = True > If you are just checking whether a pattern_id is in patterns, why not initialize > patterns as []?
Sign in to reply to this message.
"direct query" means we will get data from database instead of memory objects. 2010/6/4 <weiliu@google.com> > On 2010/06/04 01:57:41, weiliu wrote: > >> Please update cl description as I mentioned before. >> > > Sorry I saw the update of your cl description, but what does "direct > query" mean? Please make it clearer. Thanks! > > > http://codereview.appspot.com/1493041/diff/14001/15001 >> File transitfeed_editor.py (right): >> > > http://codereview.appspot.com/1493041/diff/14001/15001#newcode1172 >> transitfeed_editor.py:1172: patterns[trip.pattern_id] = True >> If you are just checking whether a pattern_id is in patterns, why not >> > initialize > >> patterns as []? >> > > > > http://codereview.appspot.com/1493041/show >
Sign in to reply to this message.
On Fri, Jun 4, 2010 at 10:14 AM, <李白> wrote: > "direct query" means we will get data from database instead of memory > objects. > I see, please update your comments then. > > 2010/6/4 <weiliu@google.com> > > On 2010/06/04 01:57:41, weiliu wrote: >> >>> Please update cl description as I mentioned before. >>> >> >> Sorry I saw the update of your cl description, but what does "direct >> query" mean? Please make it clearer. Thanks! >> >> >> http://codereview.appspot.com/1493041/diff/14001/15001 >>> File transitfeed_editor.py (right): >>> >> >> http://codereview.appspot.com/1493041/diff/14001/15001#newcode1172 >>> transitfeed_editor.py:1172: patterns[trip.pattern_id] = True >>> If you are just checking whether a pattern_id is in patterns, why not >>> >> initialize >> >>> patterns as []? >>> >> Ping? > >> >> >> http://codereview.appspot.com/1493041/show >> > > -- Best Regards, Wei Liu 86-10-62503256(o)
Sign in to reply to this message.
http://codereview.appspot.com/1493041/diff/14001/15001 File transitfeed_editor.py (right): http://codereview.appspot.com/1493041/diff/14001/15001#newcode1172 transitfeed_editor.py:1172: patterns[trip.pattern_id] = True On 2010/06/04 01:57:42, weiliu wrote: > If you are just checking whether a pattern_id is in patterns, why not initialize > patterns as []? for the pattern_id may be duplicated in the if statement, so we introduce the patterns dict to see if the pattern_id is ever used, while initialized with [] does not have the advantage to check if the pattern_id is initialized.
Sign in to reply to this message.
updated
Sign in to reply to this message.
Sign in to reply to this message.
http://codereview.appspot.com/1493041/diff/30001/31001 File transitfeed_editor.py (right): http://codereview.appspot.com/1493041/diff/30001/31001#newcode1171 transitfeed_editor.py:1171: trip.shape_desc]) How about the following? pattern_rows = {} route_id = relation[2] if (not (route_id, trip.pattern_id) in pattern_rows): pattern_rows[(route_id, trip.pattern_id)] = (trip.shape_id, trip.shape_desc)
Sign in to reply to this message.
an item like (route_id, trip.pattern_id, trip.shape_id, trip.shape_desc) will be inserted to the database. so we need an array having all of them at last. 2010/6/4 <weiliu@google.com> > > http://codereview.appspot.com/1493041/diff/30001/31001 > > File transitfeed_editor.py (right): > > http://codereview.appspot.com/1493041/diff/30001/31001#newcode1171 > transitfeed_editor.py:1171: trip.shape_desc]) > How about the following? > pattern_rows = {} > route_id = relation[2] > if (not (route_id, trip.pattern_id) in pattern_rows): > pattern_rows[(route_id, trip.pattern_id)] = (trip.shape_id, > trip.shape_desc) > > > http://codereview.appspot.com/1493041/show >
Sign in to reply to this message.
It doesn't matter, with the dict, you can still insert the key and value to the db, right? On Fri, Jun 4, 2010 at 11:21 AM, <李白> wrote: > an item like (route_id, trip.pattern_id, trip.shape_id, > trip.shape_desc) > will be inserted to the database. > so we need an array having all of them at last. > > 2010/6/4 <weiliu@google.com> > >> >> http://codereview.appspot.com/1493041/diff/30001/31001 >> >> File transitfeed_editor.py (right): >> >> http://codereview.appspot.com/1493041/diff/30001/31001#newcode1171 >> transitfeed_editor.py:1171: trip.shape_desc]) >> How about the following? >> pattern_rows = {} >> route_id = relation[2] >> if (not (route_id, trip.pattern_id) in pattern_rows): >> pattern_rows[(route_id, trip.pattern_id)] = (trip.shape_id, >> trip.shape_desc) >> >> >> http://codereview.appspot.com/1493041/show >> > > -- Best Regards, Wei Liu 86-10-62503256(o)
Sign in to reply to this message.
this may add additional loop to restructure the data to meet the database interface. 2010/6/4 Wei Liu <weiliu@google.com> > It doesn't matter, with the dict, you can still insert the key and value to > the db, right? > > > On Fri, Jun 4, 2010 at 11:21 AM, <李白> wrote: > >> an item like (route_id, trip.pattern_id, trip.shape_id, >> trip.shape_desc) >> will be inserted to the database. >> so we need an array having all of them at last. >> >> 2010/6/4 <weiliu@google.com> >> >>> >>> http://codereview.appspot.com/1493041/diff/30001/31001 >>> >>> File transitfeed_editor.py (right): >>> >>> http://codereview.appspot.com/1493041/diff/30001/31001#newcode1171 >>> transitfeed_editor.py:1171: trip.shape_desc]) >>> How about the following? >>> pattern_rows = {} >>> route_id = relation[2] >>> if (not (route_id, trip.pattern_id) in pattern_rows): >>> pattern_rows[(route_id, trip.pattern_id)] = (trip.shape_id, >>> trip.shape_desc) >>> >>> >>> http://codereview.appspot.com/1493041/show >>> >> >> > > > -- > Best Regards, > Wei Liu > 86-10-62503256(o) >
Sign in to reply to this message.
http://codereview.appspot.com/1493041/diff/30001/31001 File transitfeed_editor.py (right): http://codereview.appspot.com/1493041/diff/30001/31001#newcode1169 transitfeed_editor.py:1169: for row in pattern_rows]: Please use route_id instead of relation[2] then, which would be more readable.
Sign in to reply to this message.
updated http://codereview.appspot.com/1493041/diff/30001/31001 File transitfeed_editor.py (right): http://codereview.appspot.com/1493041/diff/30001/31001#newcode1169 transitfeed_editor.py:1169: for row in pattern_rows]: On 2010/06/04 03:32:34, weiliu wrote: > Please use route_id instead of relation[2] then, which would be more readable. Done. http://codereview.appspot.com/1493041/diff/30001/31001#newcode1171 transitfeed_editor.py:1171: trip.shape_desc]) On 2010/06/04 03:16:17, weiliu wrote: > How about the following? > pattern_rows = {} > route_id = relation[2] > if (not (route_id, trip.pattern_id) in pattern_rows): > pattern_rows[(route_id, trip.pattern_id)] = (trip.shape_id, trip.shape_desc) will add transition in InsertRows if we use dict to store the data.
Sign in to reply to this message.
LGTM http://codereview.appspot.com/1493041/diff/15002/39001 File transitfeed_editor.py (right): http://codereview.appspot.com/1493041/diff/15002/39001#newcode1165 transitfeed_editor.py:1165: Remove the empty line. http://codereview.appspot.com/1493041/diff/15002/39001#newcode1167 transitfeed_editor.py:1167: Remove the empty line.
Sign in to reply to this message.
initial version 84 in folder v2. updated to rev 85 in folder v2. 2010/6/4 <weiliu@google.com> > LGTM > > > http://codereview.appspot.com/1493041/diff/15002/39001 > > File transitfeed_editor.py (right): > > http://codereview.appspot.com/1493041/diff/15002/39001#newcode1165 > transitfeed_editor.py:1165: > Remove the empty line. > > http://codereview.appspot.com/1493041/diff/15002/39001#newcode1167 > transitfeed_editor.py:1167: > Remove the empty line. > > > http://codereview.appspot.com/1493041/show >
Sign in to reply to this message.
|
