feat(otomi normalizer) by paoinnesb · Pull Request #19 · ElotlMX/py-elotl

paoinnesb · 2024-09-20T16:59:17Z

add normalizer for Otomi with its lexc and att files. Restructure utils/fst/att and changes in makefile

name: Pull request
about:

Please include the next info:

No related issue
I have added the ortography.py for Otomi, following the convention of the Nahuatl normalizer. For this change, I also updated the Makefile, adding automation to create .att and .lexc files for the Otomi normalizer. I have restructured utils/fst/att, creating two folders (nahuatl and otomi), and added the respective orthographies for each language.
@Lguyogiro @umoqnier

…ructure utils/fst/att and changes in makefile

umoqnier

Dejo un par de comments pero en general se ve bastante bien. Buenos docstrings 💯 . Me queda duda si al final no agregamos mas tests y si corrieron los que existen para ver que todo este ok con el código existente que modificaron o que clase de pruebas hicieron para ver que no rompieran nada ❓ Si pueden agregar pantallazos a la descripción del PR para que quede documentado vendría joya :)

No mire demasiado los archivos .att o .lexc se los dejo a @Lguyogiro :p

umoqnier · 2024-09-20T23:33:19Z

elotl/otomi/orthography.py

+    _path_to_orig_fon = p
+
+_ORIG_FON_FST = ATTFST(_path_to_orig_fon)
+_available_orthographies = ['inali', 'otq', 'ots', "rfe"]


Creo que si es una variable global conviene que este en mayúsculas

Suggested change

_available_orthographies = ['inali', 'otq', 'ots', "rfe"]

_AVAILABLE_ORTHOGRAPHIES = ['inali', 'otq', 'ots', "rfe"]

elotl/otomi/orthography.py

umoqnier · 2024-09-21T01:09:26Z

elotl/otomi/orthography.py

+            print(normalized_ort + " is not a supported orthography.")
+            print("Using 'inali' as orthography.")


Creo que esto podría ir con logs como un warning ¿no?

ya quedó como warning :)

elotl/otomi/orthography.py

Lguyogiro

LGTM, se ve muy bien. Antes de hacer el merge yo recomendaría que agreguen unos tests aquí.

Lguyogiro · 2024-09-21T22:50:41Z

elotl/otomi/config.py

 #
 DEFAULT_LANG_CODE = "ote"
-SUPPORTED_LANG_CODES = ["ote"]
+SUPPORTED_LANG_CODES = ["ote", "inali", "ots", "otq", "rfe"]


creo que esto puede ser un poco confuso porque se están mezclando códigos de la lengua (como "ote") con códigos referentes al sistema de ortografía. Creo que sería mejor cambiar el nombre de este variable o mover los códigos de las ortografías a otra lista. creo que con el Nahuatl hay algo como _available_orthographies aparte de las opciones de "language codes" o algo así.

debo mencionar que lo de los "LANG_CODES" es para cuando tenemos analizadores morfológicos para distintas variantes de una lengua.

elotl/otomi/orthography.py

…LE_ORTHOGRAPHIES and DEFAULT_ORTHOGRAPHY in normalizers

…es()` in morphology files.

paoinnesb · 2024-10-05T05:16:59Z

Se hicieron los siguientes cambios

Agregar AVAILABLE_ORTHOGRAPHIES como constante en config.py y usarlo en ambos normalizadores (nahuatl y otomi), antes estaba como una variable dentro de cada orthography.py. Para el caso del los analizadores tenían un lang_code default por lo que también agregué un DEFAULT_ORTHOGRAPHY para los normalizadores dentro de config de igual manera
Encontré que el uso de logger.warn() ya es deprecated por lo que lo cambié por logger.warning dentro de los archivos orthography.py
Cambio de print() por logger.warning(). En el aviso de la ortografía por default que es usada
Cambio de path() por files(). Debido a que path quedó deprecated, lo hice en todos los archivos que encontré que usaran path()
Uso de open() cuando se abren los archivos para evitar warnings. Al correr los test en el caso de los analizadores morfológicos mandaban muchos warnings lo que hacía difícil ver cuáles eran los verdaderos errores, esto porque no se usaba la lectura de archivos con la función open() lo que ocasionaba que los archivos quedaran abiertos
Eliminación de null. En una parte de los test para el analizador morfologico del nahuatl se usaba null en vez de None, esto no permitía que el test pudiera ser ejecutado

Cambios faltantes:

Documentación para las normas del nahuatl
Test del otomí

paoinnesb · 2024-10-07T01:28:13Z

Se hicieron los siguientes cambios

Agregar AVAILABLE_ORTHOGRAPHIES como constante en config.py y usarlo en ambos normalizadores (nahuatl y otomi), antes estaba como una variable dentro de cada orthography.py. Para el caso del los analizadores tenían un lang_code default por lo que también agregué un DEFAULT_ORTHOGRAPHY para los normalizadores dentro de config de igual manera

Encontré que el uso de logger.warn() ya es deprecated por lo que lo cambié por logger.warning dentro de los archivos orthography.py

Cambio de print() por logger.warning(). En el aviso de la ortografía por default que es usada

Cambio de path() por files(). Debido a que path quedó deprecated, lo hice en todos los archivos que encontré que usaran path()

Uso de open() cuando se abren los archivos para evitar warnings. Al correr los test en el caso de los analizadores morfológicos mandaban muchos warnings lo que hacía difícil ver cuáles eran los verdaderos errores, esto porque no se usaba la lectura de archivos con la función open() lo que ocasionaba que los archivos quedaran abiertos

Eliminación de null. En una parte de los test para el analizador morfologico del nahuatl se usaba null en vez de None, esto no permitía que el test pudiera ser ejecutado

Cambios faltantes:

Documentación para las normas del nahuatl

Test del otomí

Aquí está el screenshot que muestra que las pruebas corren, hay algunas fallas en la parte del analizador del nahuatl pero tengo la teoría que estas nunca han funcionado, ya que al correrlas había un error de sintaxis, eso quedó solucionado pero aún así no pasan

umoqnier

En general está bien. Dejé unos coments con sugerencias y dudas. Resolviendo esto de mi lado estaría ok

umoqnier · 2024-10-07T02:39:05Z

elotl/nahuatl/morphology.py

 		else:
 			if lang_code not in SUPPORTED_LANG_CODES:
-				logger.error("Unsupported language variant specified.")
+				#logger.error("Unsupported language variant specified.")


¿Esto debería estar comentado?

no jeje, lo descomento

umoqnier · 2024-10-07T02:39:25Z

elotl/nahuatl/morphology.py

+		_path_to_att_dir = pkg_resources.files("elotl.nahuatl.data").joinpath(f"{self.lang_code}.mor.att")
+		_path_to_tsv_dir = pkg_resources.files("elotl.nahuatl.data").joinpath(f"{self.lang_code}.mor.tsv")


elotl/utils/fst/attapply.py

elotl/utils/morphology.py

elotl/nahuatl/orthography.py

Co-authored-by: Diego Alberto Barriga Martínez <dbarriga@ciencias.unam.mx>

Lguyogiro

LGTM

Lguyogiro · 2024-10-10T04:50:53Z

Aquí está el screenshot que muestra que las pruebas corren, hay algunas fallas en la parte del analizador del nahuatl pero tengo la teoría que estas nunca han funcionado, ya que al correrlas había un error de sintaxis, eso quedó solucionado pero aún así no pasan

Que yo me acuerde, sí funcionaban al principio pero ya se han metido muchos cambios desde que se establecieron y al parecer no siempre corríamos los tests jaja. cuando tenga tiempo me pongo a ver esto.

feat(fst): add normalizer for Otomi with its lexc and att files. Rest…

d69c445

…ructure utils/fst/att and changes in makefile

Lguyogiro self-requested a review September 20, 2024 23:30

umoqnier approved these changes Sep 21, 2024

View reviewed changes

Lguyogiro requested changes Sep 21, 2024

View reviewed changes

Paola added 3 commits October 4, 2024 21:53

change path() for files(), add warnings instead of print, add AVAILAB…

9390d66

…LE_ORTHOGRAPHIES and DEFAULT_ORTHOGRAPHY in normalizers

Use open() to avoid ResourceWarnings and replace path() with `fil…

7ac222f

…es()` in morphology files.

'None' instead of 'null'

bcffe75

paoinnesb requested a review from Lguyogiro October 5, 2024 05:14

umoqnier reviewed Oct 7, 2024

View reviewed changes

paoinnesb and others added 4 commits October 8, 2024 21:50

Update elotl/nahuatl/orthography.py

234175e

Co-authored-by: Diego Alberto Barriga Martínez <dbarriga@ciencias.unam.mx>

Update elotl/utils/fst/attapply.py

d3af68a

Co-authored-by: Diego Alberto Barriga Martínez <dbarriga@ciencias.unam.mx>

Update elotl/utils/morphology.py

0f0b02d

Co-authored-by: Diego Alberto Barriga Martínez <dbarriga@ciencias.unam.mx>

uncomment important log

75a8caf

Lguyogiro approved these changes Oct 10, 2024

View reviewed changes

umoqnier merged commit 546b447 into ElotlMX:master Oct 10, 2024

	_available_orthographies = ['inali', 'otq', 'ots', "rfe"]
	_AVAILABLE_ORTHOGRAPHIES = ['inali', 'otq', 'ots', "rfe"]

		print(normalized_ort + " is not a supported orthography.")
		print("Using 'inali' as orthography.")

		_path_to_att_dir = pkg_resources.files("elotl.nahuatl.data").joinpath(f"{self.lang_code}.mor.att")
		_path_to_tsv_dir = pkg_resources.files("elotl.nahuatl.data").joinpath(f"{self.lang_code}.mor.tsv")

Conversation

paoinnesb commented Sep 20, 2024

Uh oh!

umoqnier left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Lguyogiro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paoinnesb commented Oct 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paoinnesb commented Oct 7, 2024

Uh oh!

umoqnier left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Lguyogiro left a comment

Choose a reason for hiding this comment

Uh oh!

Lguyogiro commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

paoinnesb commented Oct 5, 2024 •

edited

Loading

Lguyogiro commented Oct 10, 2024 •

edited

Loading