Geocodificado recursivo #5
Conversation
- Drop rbind_lists_df function. - Add purrr:map_df() to rbind lists. - Minor fixes.
There was a problem hiding this comment.
Me parece una mejora muy importante la vectorización de las funciones, para poder hacer cartociudad_reverse_geocode(mi_df$latitude, mi_df$longitude) en lugar de hacer el bucle por fuera.
He añadido algunas sugerencias de cambio a la PR, comenta sobre ellas si tienes otra opinión.
Gracias por colaborar en el desarrollo del paquete, son buenos y grandes cambios :)
| municipio = info$muni, | ||
| provincia = info$province, | ||
| cod.postal = info$postalCode | ||
| ) |
There was a problem hiding this comment.
Sobre la eliminación de esto, creo que conviene mantener en el paquete el mapeo de nombres, para:
- Mantener coherencia: son nombres con mismo idioma, mismo formato, no como los que devuelve directamente la API (hay inglés, español, underscored, camelCased, ...)
- Mantener retro-compatibilidad con los usuarios actuales
- Asegurar futura compatibilidad: si se lanza una nueva versión de la API y los campos tienen otros nombres, cambiamos el mapeo en el paquete, y la actualización será transparente para los usuarios
Dime cómo lo ves. Si estás de acuerdo, sería bueno mantener el trozo de documentación eliminado sobre los nombres
There was a problem hiding this comment.
Me parece perfecto, aunque creo que sería mejor devolver un data.frame en lugar de una lista. ¿Conservamos esa parte?
There was a problem hiding this comment.
OK, totalmente de acuerdo, tiene más sentido ahora que está vectorizado
| #' cartociudad_reverse_geocode(40.473219, -3.7227241) | ||
| #' | ||
| #' # Query multiple points | ||
| #' cartociudad_reverse_geocode(c(40.473219, 39.46979), c(-3.7227241, -0.376963)) | ||
| #' |
There was a problem hiding this comment.
Como recomendación, estaría bien añadir un test en tests/testthat.R con el caso de llamar a cartociudad_reverse_geocode para múltiples localizaciones
There was a problem hiding this comment.
Yo me ocupo de las pruebas.
He pensado en comprobar el funcionamiento ante casos típicos que estoy viendo, como una dirección sin número (devuelve la geometría de toda la vía y la latitud y longitud del portal 0), portales y pk's inexistentes (devolución del punto más próximo), y viales inexistentes.
| #' | ||
| #' # Query multiple addresses | ||
| #' address <- c(address, "plaza del ayunamiento 1, valencia") | ||
| #' my.address <- cartociudad_geocode(full_address = address) |
There was a problem hiding this comment.
Misma recomendación que con cartociudad_reverse_geocode: sugiero incluir un test para múltiples direcciones
DESCRIPTION
Outdated
| @@ -9,7 +9,7 @@ Authors@R: c(person("Carlos J.", "Gil Bellosta", email="[email protected]", r | |||
| Author: Carlos J. Gil Bellosta, Luz Frías | |||
| Maintainer: Carlos J. Gil Bellosta <[email protected]> | |||
| Description: Access to Cartociudad cartography API, which provides mapping and other related services for Spain. | |||
| Imports: httr, jsonlite, xml2, plyr, geosphere | |||
| Imports: httr, jsonlite, xml2, plyr, geosphere, purrr | |||
There was a problem hiding this comment.
No sé si el uso actual justifica incluir una nueva dependencia (purrr). Actualmente solo se utiliza para hacer 2x rbind de lista de data.frames, que se puede sustituir por funciones de R base: do.call(rbind, res_list)
There was a problem hiding this comment.
Sí, en principio lo hice así. He de reconocer que soy un adicto al paquete purrr... :) Lo corrijo.
There was a problem hiding this comment.
He pensado en usar plyr::rbind.fill(), puesto que los data.frame's pueden tener diferente número de columnas.
- Remove purrr from Imports: purrr:map_df to plyr::rbind.fill. - Original names in results from cartociudad_reverse_geocode.
|
Cambios realizados. He subido a versión 0.5.3 |
Add progress bar to cartociudad_geocode & cartociudad_reverse_geocode. Add utils to Imports. Add new argument to cartociudad_geocode to use the previous geocoder version and adapt code to it.
There was a problem hiding this comment.
Tengo que probar el nuevo código que mandas. De todos modos, ¿por qué eliminas la referencia catastral (y otras partes de la salida)? Es muy útil en muchas aplicaciones.
Te comento también que vamos a hacer cambios en la manera de organizar la gestión de este y otros paquetes similares pronto. Por eso hemos tardado en responder. Dado tu interés, te tendremos informado.
|
Hola, En principio el código es funcional, aunque he de reconocer que en este tiempo lo he ido adaptando para afrontar los problemas que me iban surgiendo en el día a día, todos ellos relacionados con el cambio de versión del geocodificador: ha ido a peor en varios sentidos (incluyendo problemas con el servidor para algunas direcciones concretas o una disminución considerable en el número de geocodificaciones exitosas), lo que ha provocado que incorpore una opción para consultar la versión previa. Por ejemplo, y en relación con tu consulta, la nueva versión del geocodificador no devuelve la referencia catastral, así que si se utiliza esa versión el resultado es un valor perdido en ese campo. Esta divergencia de campos devueltos ha hecho que tratase de unificar la salida que devuelve la función, incorporando un campo que indica la versión del geocodificador. No obstante, la nueva versión incluye algunas mejoras, como el correcto manejo de puntos kilométricos o un comportamiento consistente al devolver los numeros de vía pares o impares más próximos en consultas donde no se encuentren los puntos exactos (en la versión previa cambiaba de par a impar o viceversa en un porcentaje elevado, creo que cercano al 40 %). Gracias por mantenerme informado: estoy interesado en colaborar en la medida de lo posible, ya que nuestro grupo está utilizando el servicio de geocodificado de forma intensiva y nos interesa que sea lo más funcional posible. Un saludo! :) |
Hola de nuevo!
He estado revisando las funciones y parece que los resultados son prácticamente idénticos a la versión previa (PR #3).
No obstante, me he fijado en que las funciones eran algo engorrosas a la hora de introducirlas dentro de un bucle, de forma que he implementado esta característica como parte de las funciones
cartociudad_geocode()ycartociudad_reverse_geocode(). Concretamente, los cambios son:data.framecon la consulta y valores perdidos para el resto de elementos.Un saludo! :)