elxinoconequis / seleccion_analista_2022

🐍 Joaquín Fernando Ortega Silva
GNU Affero General Public License v3.0
0 stars 0 forks source link

Crear branch 'restore' en commit dcf4d4b #18

Closed elxinoconequis closed 2 years ago

elxinoconequis commented 2 years ago

Realizado con reset --soft; se agrego los cambios en dummy_model.py, test_dummy_model.py y mypy.ini

Eliminar comentarios de estructura en dummy_model.py

elxinoconequis commented 2 years ago

@devarops Buenas tardes, he realizado esta rama de acuerdo a lo instruido, haciendo el branch de acuerdo a lo que dijiste en el PR #17 . Creo que encontraras que esta mucho más limpia. Y con esto he agregado la modificación a las funciones que quería en el branch develop

elxinoconequis commented 2 years ago

Moví el Actions a tu fork.

¿Por favor podrías hacer pasar el Actions

Además, aquí te dejo unos comentarios.

Sé que el Actions corre una vez cuando hago push a mi repo y otra cuando se abre un PR, ¿de preferencia hasta que pase todas las pruebas abro el PR?.

Tengo curiosidad de saber si ustedes acostumbran hacer los Action de manera local - porque a primera vista se ve que lo común es hacerlo desde el fork, pues no es tan directo correr Actions localmente - como menciona esta pregunta de Stack Overflow

devarops commented 2 years ago

Sí acostumbramos correr el Actions de manera local. Aquí puedes ver que en este examen de selección, (la segunda mitad de) el Actions es equivalente a:

make check
make coverage
make mutants

image

Si corres esos tres comandos localmente (dentro del contenedor) es igual que correr el Actions localmente.

¿de preferencia hasta que pase todas las pruebas abro el PR?

No, no te tienes que esperar a que pase el Actions para abrir el PR. Es preferible que abres el PR lo antes posible para que te podamos ayudar a pasar el Actions. En este caso está tronando el make check, y está ofreciendo mensajes de error muy claros.

pollos_petrel/dummy_model.py:57:24: E712 comparison to True should be 'if cond is True:' or 'if cond:'
pollos_petrel/dummy_model.py:60:26: E711 comparison to None should be 'if cond is None:'
elxinoconequis commented 2 years ago

Sí acostumbramos correr el Actions de manera local. Aquí puedes ver que en este examen de selección, (la segunda mitad de) el Actions es equivalente a:

make check
make coverage
make mutants

Si corres esos tres comandos localmente (dentro del contenedor) es igual que correr el Actions localmente.

¿de preferencia hasta que pase todas las pruebas abro el PR?

No, no te tienes que esperar a que pase el Actions para abrir el PR. Es preferible que abres el PR lo antes posible para que te podamos ayudar a pasar el Actions. En este caso está tronando el make check, y está ofreciendo mensajes de error muy claros.

pollos_petrel/dummy_model.py:57:24: E712 comparison to True should be 'if cond is True:' or 'if cond:'
pollos_petrel/dummy_model.py:60:26: E711 comparison to None should be 'if cond is None:'

Tienes razón, hice el make check y me da esos mensajes, localmente ya hice las correcciones que me pidió el make check.

Por curiosidad corrí el make coverage y veo que genera muchos archivos (los cuales ya revertí localmente), pero no lo hace muy atractivo para mi porque creo que puede hacer cambios que no deseo. ¿Que hace este comando? En el Makefile solo veo que dice esto coverage: coverage_python coverage_r

Y bueno make mutants ya estoy familizarizado con el.

devarops commented 2 years ago

El make coverage no lo necesitarás en Python; es algo que usamos en R. Con el make check y el make mutants es suficiente para Python. Por lo pronto, puedes ignorar el make coverage.

elxinoconequis commented 2 years ago

Buenas tarde, he estado resolviendo los mutantes y observé algo muy interesante (para mí, por supuesto) y esto es que he visto como la mutación en las asignaciones es letal, por ejemplo si tuviera una variable intermedia (para facilitar la lectura del código) esta muta en la asignación, entonces para evitar mutantes pues en este caso conviene cortar al intermediario, por ejemplo:

check_NaN = df_raw.isnull().values.any()

La cual regrese un valor True o False, justo después lo metía a un if de la siguiente manera

if check_NaN is True: hace algo else: hace otra cosa

Entonces esto me da doble posibilidad de generar mutantes, una en la asignación y otra con el ..is True.

Entonces corte el is True para evitar mutaciones, por que si es booleano el if me lo detecta automáticamente.

Entonces, con esta última modificación y la eliminación del intermediario, eliminé varios mutantes de esta naturaleza en el código

Únicamente me queda un mutante en la linea 57, y no entiendo como le puedo pasar un cero sin que mute.

`df_clean = df_raw.fillna(0)

El default del método .fillna() es un None. Una de las cosas que se me ocurre es hacer una función que me cuente el número y posición de los NaN en el dataframe y que con esta información agregar al test una assertpara ver si concuerda un cero con la posición. Es decir que pueda diferenciar, entre un cero que viene del .csv original y una sustitución de cero hecha por mi función de limpiar el dataframe clean_NA(dataframe) `

devarops commented 2 years ago

@elxinoconequis Son muy interesantes tus observaciones.

Creo que te estás esforzando por eliminar los 👾 modificando pollos_petrel/dummy_model.py. Pero eso será muy difícil y reducirá la legibilidad del código. La forma en la que debes cazar los 👾 es modificando tests/test_dummy_model.py.

Entiendo los 👾 se encuentran en pollos_petrel/dummy_model.py, pero nos están describiendo los cambios requeridos en tests/test_dummy_model.py.

Los 👾 que se encuentran en pollos_petrel/*.py se cazan modificando tests/test_*.py

elxinoconequis commented 2 years ago

@elxinoconequis Son muy interesantes tus observaciones.

Creo que te estás esforzando por eliminar los space_invader modificando pollos_petrel/dummy_model.py. Pero eso será muy difícil y reducirá la legibilidad del código. La forma en la que debes cazar los space_invader es modificando tests/test_dummy_model.py.

Entiendo los space_invader se encuentran en pollos_petrel/dummy_model.py, pero nos están describiendo los cambios requeridos en tests/test_dummy_model.py.

Los space_invader que se encuentran en pollos_petrel/*.py se cazan modificando tests/test_*.py

Si, entiendo que se debe cazar desde los test_*, sin embargo resulta fácil también cambiarlo desde el código de producción, y siento que a su vez esto ayuda a optimizar el código (en términos de rendimiento).

Aunque sé que debo cazar los mutantes desde los test, esta un poco oscurecido para mí la relación que hace pytest con mutmut; mi intución me dice que mutmut decide hacer las mutaciones de acuerdo a la información que le da los test. Ambas librerías hacen pruebas unitarias, y creo que la diferencia entre una y otra radica en que mutmut busca obligarnos a crear una prueba, aunque hayamos pasado todas las pruebas de pytest.

Mi suposición es que parte de la información de los assert es lo que entra mutmut, porque no es tan aleatorio donde decide mutar.

Por ahora experimentaré hacer una prueba para el método .fillna() y ver si finalmente puedo cazar el :space_invader: . Aprovecho para reorganizar las líneas que me pediste.

devarops commented 2 years ago

Si, entiendo que se debe cazar desde los test_*, sin embargo resulta fácil también cambiarlo desde el código de producción, y siento que a su vez esto ayuda a optimizar el código (en términos de rendimiento).

En nuestro equipo siempre preferimos legibilidad sobre optimización (rendimiento). Además, hay algunos cambios en el código de producción que simplemente no son posibles porque no siguen nuestras guías de estilo. Checa #19

creo que la diferencia entre una y otra radica en que mutmut busca obligarnos a crear una prueba, aunque hayamos pasado todas las pruebas de pytest.

Correcto. Diste en el clavo. 🎯 El pytest nos dice si nuestras pruebas pasan mientras que mutmut nos dice si nuestras pruebas son suficientes.

Mi suposición es que parte de la información de los assert es lo que entra mutmut, porque no es tan aleatorio donde decide mutar.

En realidad el mutmut no obtiene información de pytest. Sólo hace mutaciones aleatorias (dentro de una lista predeterminada) al código de producción sin ver las pruebas. Lo único que espera el mutmut es que las pruebas truenen después de una mutación, pero no le importa cuál prueba tronó; mutmut no ve las pruebas. Si después de hacer una mutación las pruebas pasan, entonces tenemos 👾 sobrevivientes. Si después de hacer una 👾 las pruebas truenan, entonces cazamos 🏹 al mutante.

Por ahora experimentaré hacer una prueba para el método .fillna() y ver si finalmente puedo cazar el 👾 . Aprovecho para reorganizar las líneas que me pediste.

Me parce buena idea. Mañana intentaré hacer un 📽️ para echarte la mano.

devarops commented 2 years ago

@elxinoconequis ¿Por favor podrías presionar el botón Re-request review cuando estés listo para mi revisión?

elxinoconequis commented 2 years ago

@elxinoconequis ¿Por favor podrías presionar el botón Re-request review cuando estés listo para mi revisión?

Si, solo que cunado te dije me emocioné e hice el push antes hacer el make check y make format, pero ya esta en el actions. Ahorita hago re-request review.

devarops commented 2 years ago

Si, solo que cuando te dije me emocioné e hice el push antes hacer el make check y make format, pero ya está en el actions. Ahorita hago re-request review.

Sí, a todos se nos olvida. Me pasa muy seguido 😅

Por suerte al Actions no le falla la memoria 😆

devarops commented 2 years ago

Parece que el Actions sigue fallando.

¿Por favor podrías checarlo?

@elxinoconequis Ignora el mensaje anterior. Me equivoqué. El Actions sí está pasando. 👍🏾

En un momento te paso mi revisión

elxinoconequis commented 2 years ago

Parece que el Actions sigue fallando. ¿Por favor podrías checarlo?

@elxinoconequis Ignora el mensaje anterior. Me equivoqué. El Actions sí está pasando. 👍🏾

En un momento te paso mi revisión

:relieved: Si, muchas gracias.