ropensci / bikedata

:bike: Extract data from public hire bicycle systems
https://docs.ropensci.org/bikedata
81 stars 16 forks source link

lat-lng station patch #83

Closed tbuckl closed 6 years ago

tbuckl commented 6 years ago

seems like our old friends long and lat were inverted in a query.

i made this change and then rebuilt and checked against sf, lo, and ny using the following:

check_station_location <- function(cityname, long_check) {
  data_dir=tempdir()
  bikedb <- file.path (data_dir, paste0('db',cityname))
  dl_bikedata(cityname,dates=201802:201803, data_dir = data_dir)
  store_bikedata(city=cityname,data_dir=data_dir,bikedb)
  stations <- bike_stations(bikedb)
  m_lon = median(stations$longitude)
  check = (m_lon > (long_check[1]) & 
             m_lon < (long_check[2]))
  return(check)
}
check_station_location('lo',c(-10,10))

unfortunately the test above is not robust against introducing a new city. the following seems to re-load london.

check_station_location('sf',c(-100,-130))

is there a better way to instantiate a blank tempdir for the data without requiring a user to input y/n for download and directory creation?

codecov-io commented 6 years ago

Codecov Report

Merging #83 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #83   +/-   ##
=======================================
  Coverage   79.45%   79.45%           
=======================================
  Files          18       18           
  Lines        2161     2161           
=======================================
  Hits         1717     1717           
  Misses        444      444
Impacted Files Coverage Δ
src/read-station-files.cpp 85.55% <100%> (ø) :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 8ad31ae...8c595dd. Read the comment docs.

mpadge commented 6 years ago

Yep, that was it alright - thanks! The pattern was set in read-city-files.cpp, which had the following lines:

(*stationqry)[stn_id] = "(\'" + city + "\',\'" + stn_id + "\',\'" +
    stn_name + "\'," + lon + "," + lat + ")";

so the former version:

std::string fullstationqry = "INSERT OR IGNORE INTO stations "                  
        "(city, stn_id, name, latitude, longitude) VALUES ";

was definitely wrong. Should be all good now