Solution Christian #2
Conversation
Feature/challenge 1
Feature/challenge 2
feat(dockerize): Creating dockerfile for create docker image
doc(solution): Included comments in main Readme.md
| """ | ||
|
|
||
| # Paths | ||
| RAW_FILE = "data/raw/listings.csv" |
There was a problem hiding this comment.
Intenta evitar poner paths relativos (o completos). Puedes usar Pathlib para generar paths que se ajusten a todos los entornos
| @@ -0,0 +1,180 @@ | |||
| """ | |||
There was a problem hiding this comment.
¿No ves más interesante desarrollar este script como una clase preprocess? El objetivo es tener un objeto data_preprocessing donde podamos ejecutar los métodos necesarios.
| @@ -0,0 +1,134 @@ | |||
| """ | |||
| This file contains all test for preprocess.py | |||
| """ | |||
There was a problem hiding this comment.
Si lo planteamos como clase, también podemos testear no solo que los métodos individualmente funcionan sino que el output es el esperado (que todos los métodos están funcionando bien juntos). Por ejemplo, si nos equivocamos en uno de los métodos y el código se carga todas las columnas salvo la que procesamos, el test sigue pasando, y todos los demás también. Si lo miramos en conjunto podemos ver que falla.
|
|
||
| class AppSettings(BaseSettings): | ||
| app_name: str = "Inference API" | ||
| model_path: str = "models/random_forest_classifier_2022-08-04 08:31:07.734769.pkl" |
There was a problem hiding this comment.
Hubiese sido interesante hacer uso del f-string para que podamos pasar la fecha del modelo como parámetro (en Docker por ejemplo)
| inference_engine = InferenceEngine(settings=settings) | ||
| app = FastAPI() | ||
|
|
||
|
|
There was a problem hiding this comment.
Un punto extra hubiese sido meter algo de security, como un API token simple.
| try: | ||
| neighbourhood = self.__settings.mapping_columns["neighbourhood"][request.neighbourhood] | ||
| except KeyError as key_exc: | ||
| raise HTTPException(status_code=400, detail="Neighbourhood not valid") | ||
|
|
||
| try: | ||
| room_type = self.__settings.mapping_columns["room_type"][request.room_type] | ||
| except KeyError as key_exc: | ||
| raise HTTPException(status_code=400, detail="Room type not valid") |
There was a problem hiding this comment.
Igual hubiese valido la pena escribir una función para esto ya que parece que el error es el mismo.
| @@ -0,0 +1,2 @@ | |||
| #! /bin/bash | |||
| python -m uvicorn api.app:app --host 0.0.0.0 --port 80 | |||
There was a problem hiding this comment.
Más que un cambio es que puedes lanzar este mismo comando desde Docker, por evitar más dependencias de código.
| @@ -0,0 +1,17 @@ | |||
| FROM python:3.9 | |||
|
|
|||
| ENV PYTHONPATH=lab | |||
There was a problem hiding this comment.
Puedes añadir ENV PYTHONUNBUFFERED=1 para forzar que el stdout salga al terminal (recomendado cuando corremos Python en Docker)
Solution for MLE challenge