search

ITSLeeds / UK2GTFS

Convert UK transport data (TransXchange / ATOC CIF) to GTFS format in R
https://itsleeds.github.io/UK2GTFS/
GNU General Public License v3.0
37 stars 13 forks source link

Oo tfwm performance #53

Closed oweno-tfwm closed 1 year ago

oweno-tfwm commented 1 year ago

spelling mistake correction and performance improvements - see comments on individual commits

mem48 commented 1 year ago

This mostly looks good, but what is the point of

DURATION_INDEX <- match("duration", CHECKROWS_NAME_VECTOR)
START_DATE_INDEX <- match("start_date", CHECKROWS_NAME_VECTOR)
END_DATE_INDEX <- match("end_date", CHECKROWS_NAME_VECTOR)
MONDAY_INDEX <- match("monday", CHECKROWS_NAME_VECTOR)
SUNDAY_INDEX <- match("sunday", CHECKROWS_NAME_VECTOR)
oweno-tfwm commented 1 year ago

Hi Malcolm,

Just initialising some constants - but doing it in such a way that if the vector of names is altered in the future it doesn't rely on someone also keeping some numeric constants up to date too.

I'm from a C/C++/Java background and new to R so I'm not totally up to speed on R conventions, and don't know how good the optimiser in R is, hence was conservative and put the constant initialisation as a global rather than inside the fn.

Cheers Owen

Owen O'Neill Senior Research Analyst

Confidentiality: The information in this email may be confidential, contain personal and/or sensitive information, and/or may be legally privileged. If an addressing or transmission error has misdirected this email, please notify the author by replying to this email and then deleting the original and your reply. If you have received this email in error, any disclosure, copying, distribution, or any action taken or omitted to be taken in reliance on the email, may be prohibited and potentially unlawful. Any views or opinions expressed in this email are those of the sender and do not necessarily reflect the views of West Midlands Combined Authority, unless explicitly stated otherwise.

Find out about WMCA by visiting https://www.wmca.org.uk

6 Please consider the environment, before printing this email. From: Malcolm Morgan @.> Sent: Wednesday, August 9, 2023 9:18 PM To: ITSLeeds/UK2GTFS @.> Cc: Owen O'Neill @.>; Author @.> Subject: Re: [ITSLeeds/UK2GTFS] Oo tfwm performance (PR #53)

This mostly looks good, but what is the point of

DURATION_INDEX <- match("duration", CHECKROWS_NAME_VECTOR)

START_DATE_INDEX <- match("start_date", CHECKROWS_NAME_VECTOR)

END_DATE_INDEX <- match("end_date", CHECKROWS_NAME_VECTOR)

MONDAY_INDEX <- match("monday", CHECKROWS_NAME_VECTOR)

SUNDAY_INDEX <- match("sunday", CHECKROWS_NAME_VECTOR)

- Reply to this email directly, view it on GitHubhttps://ddec1-0-en-ctp.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fgithub.com%2fITSLeeds%2fUK2GTFS%2fpull%2f53%23issuecomment%2d1672087320&umid=1a62c2b9-43d5-44a6-9204-2b4374a0f1e1&auth=ed58b55b1e1bbd0f27838022b24e687c0907dabf-5a10453ff83ea0af933fec08f0d3627492abcbc3, or unsubscribehttps://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fgithub.com%2fnotifications%2funsubscribe%2dauth%2fBBUESVUN3QQ763CQ34GXEMDXUPV7XANCNFSM6AAAAAA3KEQUWQ&umid=39b0c475-2f83-4111-925a-3c80ad556ccd&auth=8408d096bf03c3362b54de716d3bb413afe8736a-6be8472841c1d92575ada88b59ee5f3b4a1f7424. You are receiving this because you authored the thread.Message ID: @.***>

mem48 commented 1 year ago

Probably won't make any difference, but won't hurt.

PR approved