Move variable creation to pct-load

[usr110]

We have gone through this before but just to summarize: Two variables: id for rnet and rqincr for r_quiet should be once created in pct-load; not on the fly in pct-shiny

Affected code would be: https://github.com/npct/pct-shiny/blob/master/server-base.R#L56-L70

Creation of the id variable for london takes almost half a second (using fast internet connection), which is not ideal.

[Robinlovelace]

Good point - that's one to update in pct-data (via pct-load), alongside updating the distance variable in rf and rq.

[usr110]

Updated rqincr's name to rq_incr, as we are using snake_case for variable names. Once the new set of data is generated for pct-data, using pct-load, then I will change the affected code in the application.

[Robinlovelace]

:+1: re naming conventions

[Robinlovelace]

Think this is sorted - pls re-open if needed @usr110

[usr110]

I am afraid the issue is not resolved yet. Although rq_incr is there in build_region.R under pct-load (see https://github.com/npct/pct-load/blob/master/build_region.R#L88) but it doesn't appear in the variable list.

See the summary for avon:

colnames(readRDS("../pct-data/avon/rq.Rds")@data) [1] "id" "length" "change_elev" "av_incline" "time"

[Robinlovelace]

Verified, with the slightly more concise:

names(readRDS("../pct-data/avon/rq.Rds"))
## [1] "id"          "length"      "change_elev" "av_incline"  "time"    

So to clarify, there should be a new variable in rq for the quietness diversion factor in pct-load?

Thanks for re-opening - this is something we can add after the national build is completed and definitely not a blocker to the same extent that some of the unbuilt regions are.

One to note for the lsoa build @mem48.

[usr110]

Yes, you're right. What I fail to understand is that if most of the regions were built in the last 6 days (assuming using build_region.R), then why doesn't this variable exist?

[Robinlovelace]

Because it's not in the codebook - that how we decide what people see: https://github.com/npct/pct-shiny/blob/master/static/codebook_routes.csv

[Robinlovelace]

And what's kept in the build process - a problematic duality there...

[usr110]

The codebook variables are not used in pct-load any more - https://github.com/npct/pct-load/commit/5e1471d94e0c13e77baef79cd99c0515c06fbcd0

Perhaps the data was generated before this commit.

[Robinlovelace]

Yes - so the codebook now determines what people download. It's determined here, so you still need to add it to the codebook (and the relevant files in pct-bigdata and pct-data) here to fix this issues:

https://github.com/npct/pct-shiny/pull/451/commits/ffe6add56acda8e6b69aa1f46619356ef49598e0#diff-f89beddd12f7a4f7a2203ce282115a66R629

[Robinlovelace]

This is the relevant commit: https://github.com/npct/pct-shiny/commit/40eb783dea094119b4901c72b547a3ca6b385120

[usr110]

That makes sense, thanks.

[Robinlovelace]

Not sure this actually fixes it - was referring to code in pct-load.

But no issue now and we can always re-open if it becomes an issue.

[usr110]

All the changes, if I remember correctly, have been made in pct-load but since the changes the data has not been produced. So in fact the issue has not been resolved.

[nikolai-b]

I believe this to be fixed, I changed to code to use the pre-calculated variable in that commit.

[usr110]

I saw that you updated the variable names, Nikolai, but this issue will be fixed when the new data is going to be generated (using pct-load). Had to revert your commit as route-network wasn't working (as it didn't have the required id variable).

[AnnaGoodman1]

I think now fixed in pct-tidy