cyipt / acton

Active Transport Options for New Developments
https://cyipt.github.io/acton/
GNU General Public License v3.0
3 stars 2 forks source link

Add arguments to get_planit_data function #15

Closed Robinlovelace closed 4 years ago

codecov-io commented 4 years ago

Codecov Report

Merging #15 into master will not change coverage. The diff coverage is 0%.

Impacted file tree graph

@@         Coverage Diff          @@
##           master   #15   +/-   ##
====================================
  Coverage       0%    0%           
====================================
  Files           3     3           
  Lines          17    25    +8     
====================================
- Misses         17    25    +8
Impacted Files Coverage Δ
R/planit.R 0% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e974789...9fa8fb6. Read the comment docs.

Robinlovelace commented 4 years ago

Heads-up @joeytalbot could you review this PR? Can explain in person later today.

Robinlovelace commented 4 years ago

It might be worth renaming start_date to something like date_of_application, so it's clear exactly what this refers to.

Could be worth it but I think the +s of being 100% faithful to the names in the planit api outweigh the +s of clarity that it's the date of application, not project build. May be worth mentioning in the documentation, e.g. on this line: https://github.com/cyipt/acton/pull/15/files#diff-8fb302eb411c911c5edf99be692ad05cR15 which could be changed to

 #' @param start_date The earliest application (date of application) to be filtered `"2000-02-01"` 

Sound like a plan?

Robinlovelace commented 4 years ago

We also need to add fields for the application status / decision and the decision date.

Indeed, can do that in future.

Robinlovelace commented 4 years ago

Heads-up @joeytalbot I've just updated the description of the start_date parameter.

Robinlovelace commented 4 years ago

We also need to add fields for the application status / decision and the decision date.

Is that worth adding an issue on that? I think that's something that will need to be done on the planit side - we should definitely update the package if/when the api provides those fields. These are fields we have so far:

library(acton)
> bbox = c(-1.4, 53.7, -1.3, 53.8)
> res = get_planit_data(bbox) # return geographic (`sf`) object
Getting data from https://www.planit.org.uk/api/applics/geojson?limit=6&bbox=-1.4%2C53.7%2C-1.3%2C53.8&end_date=2020-01-09&start_date=2000-02-01&pg_sz=6
> class(res)
[1] "sf"         "tbl_df"     "tbl"        "data.frame"
> names(res)
 [1] "doc_type"       "name"           "url"            "description"    "when_updated"  
 [6] "authority_id"   "source_url"     "authority_name" "link"           "postcode"      
[11] "address"        "lat"            "lng"            "start_date"     "uid"           
[16] "altid"          "geometry"