[W3-pipelines-project] Fran Leston

FranLeston commented 3 years ago

W3-pipeline-project

breogann commented 3 years ago

Hola Fran! 🙋🏻‍♂️

Como siempre digo: damos todo el feedback posible, pero eso no quiere decir que esté mal.

El proyecto tenía dos objetivos: enriquecer datos & hacer un pipeline.

Para el primero: utilizar una API o web scrapping para relacionar los datos.
Para el segundo: que haya un pipeline quiere decir que el resultado de una función fuera la entrada de otra. Es decir, que ejecutando una acción, los datos se procesen de varias formas para devolver un output diferente al inicial. Aquí puedes hacer un ejecutable donde el resultado de la primera función (la descarga del dataset) sea la entrada de la(s) segunda(s) funcion(es) de limpieza. El flujo de ejecución sería:

ruta > df_descargado > df_limpio

Has utilizado:

Dataset original: NBA players
Enriquecido con: scrapping/API
El README.md ✍🏻: súper bien explicado. Muchas veces dejamos de lado las instrucciones para que alguien pueda utilizar el código. Explicas el objetivo, el proceso que sigue tu código y cómo ejecutarlo.
Respecto a la organización del repositorio 📚
- El .gitignore: no se te ha colado ningún archivo temporal ni variables de entorno.
- El main.py está perfecto en la root.
- Los jupyter y los archivos de src también con nombres significativos 👌
El ejecutable ⚡️: está genial. Has llamado a funciones importadas de otros sitios.
- Tienes una función con el return vacío?

Respecto al código 👩‍💻:

En el src:

Esto quizá lo puedas refactorizar:

try:
    player_minutes = int(ele['statistics'][0]['games']['minutes'])
except:
    player_minutes = 0

try:
    player_goals = int(ele['statistics'][0]['goals']['total'])
except:
    player_goals = 0

try:
    player_assists = int(ele['statistics'][0]['goals']['assists'])
except:
    player_assists = 0

try:
    player_saves = int(ele['statistics'][0]['goals']['saves'])
except:
    player_saves = 0

que sólo te cambia lo último. Si guardas eso en una lista e iteras; o metes en una mini-función y puedes hacer un solo try/except.

Lo mismo para esto:

if player_id == 154:
                player_twitter_username = "TeamMessi"
            elif player_id == 136:
                player_twitter_username = "3gerardpique"
            elif player_id == 738:
                player_twitter_username = "SergioRamos"
            elif player_id == 144:
                player_twitter_username = "5sergiob"
            elif player_id == 743:
                player_twitter_username = "MarceloM12"
            elif player_id == 137:
                player_twitter_username = "SergiRoberto10"
            elif player_id == 139:
                player_twitter_username = "samumtiti"
            elif player_id == 157:
                player_twitter_username = "LuisSuarez9"
            elif player_id == 128:
                player_twitter_username = "JordiAlba"
            elif player_id == 759:
                player_twitter_username = "Benzema"
            elif player_id == 752:
                player_twitter_username = "ToniKroos"
            else:
                player_twitter_username = "None"

está pidiendo a gritos un diccionario.

Genial por haber incluido los errores con nombre.
Top trabajar con nan:
```
def clean_weight(weight):
    if weight == "None":
        return np.nan
```
puede parecer algo obvio, pero la gente suele devolver strings.

En los jupyters:
- Está muy bien la visualización! algún gráfico como el último quizá lo quitaría o lo explicaría de una forma más explícita.

Estaría bien un jupyter a modo de story-telling con simplemente títulos, gráficas y una explicación pequeña (sin código), a modo de presentación-resumen.

Como resumen:

Intenta hacer funciones más cortas
Incluye los docstrings
Has utilizado un montón de error handling
Has dado instrucciones sobre como ejecutar tu mismo código
Es un proyectazo, enhorabuena por crear una bbdd con SQL 👏

Te digo cosas a cambiar/mejorar (porque te las tengo que decir) pero está súper bien Fran, es un trabajazo, así que enhorabuena 💪

FranLeston commented 3 years ago

Gracias por el feedback Fer!

On Tue, Apr 20, 2021 at 4:52 PM Fernando Costa @.***> wrote:

Closed #2 https://github.com/Ironhack-Data-Madrid-Marzo-2021/W3-pipelines-project/issues/2 .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Ironhack-Data-Madrid-Marzo-2021/W3-pipelines-project/issues/2#event-4619757573, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4U4ZOSDYVJ4EOMDBPXPPLTJWIL7ANCNFSM42ZEUNWQ .

Ironhack-Data-Madrid-Marzo-2021 / W3-pipelines-project

[W3-pipelines-project] Fran Leston #2