Kelabdi / bcn-data-api

Streamlit BCN city data visualization API
1 stars 0 forks source link

Corrección #1

Open ferrero-felipe opened 3 years ago

ferrero-felipe commented 3 years ago

¡Olé Karim! 



Muy buen trabajo, como ya hemos visto en tu presentación. La corrección será de la siguiente manera, revisaré todo tu código y iré comentando una série de cosas que vaya viendo. Al final, haré apuntes generales.

Muy buen trabajo en la API, todo muy claro en el código, bien modulado y bien organizado. Los endpoints son muy descriptivos y serían de fácil uso. Lo único que te comento es que podrías evitar algunas repeticiones parametrizando mejor las funciones, por ejemplo en los endpoints /bcn-unemployed-by-district y /bcn-unemployed-by-neighborhood. ;)

El código de tu dashboard podría beneficiarse de una estructuración similar, en lugar de tener mucho en el fichero principal. Lo que no entiendo es por qué cargas los csv con pandas y también haces peticiones a la API. No es información redundante? 



La impresión que tengo es que el csv que cargas solo sirve para pintar el dataframe. En ese caso no es muy eficiente tener que cargarle a cada vez que la página se ejecuta. Quizás una buena salida seria definir una función para cargar el dataset y decorarla con @st.cache. 



No te voy decir que está mal cargar el dataframe del csv. Es una posibilidad igual, pero en ese caso nuestro objetivo era separar bien los servicios. :)

Has hecho un muy buen proyecto!


image

Kelabdi commented 3 years ago

Vaya, es un comentario publico, como es personal voy a borrarlo, imagino que se te avisara por correo con el mensaje.