Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Take subject='abbrev' into account again #506

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

percevalw
Copy link

Hi ! First, thank you for this great package !

This PR aims at fixing the abbrev mode of subjects when running .conjugate on a verb.

Currently (Python 3.11, mlconjug3==3.11.0), running the first example of the docs:

from mlconjug3 import Conjugator

# initialize the conjugator
conjugator = Conjugator()

# conjugate the verb "parler"
verb = conjugator.conjugate("parler")
# OR verb = conjugator.conjugate("parler", subject="abbrev")

# print all the conjugated forms as a list of tuples.
print(verb.iterate())

print(verb["Indicatif"]["Présent"]["1s"])

raises a KeyError: '1s', because it runs VerbXX._load_conjug twice
(without, store the results in the full_forms attribute, then with abbreviations),
and dismisses the second call, therefore not populating the abbreviated forms of subjects.

This was not the past behavior (at least in 3.9.0), and may cause issues in dependent packages (at least edsnlp)

This can be fixed by assigning the full_forms dict into the conjug_info attribute of verbs before perfoming
the first call.

@SekouDiaoNlp
Copy link
Collaborator

Hi @percevalw , thank you for submitting the PR.

I have been pretty busy lately but now have time to dedicate to this project.

i will review your changes shortly and let you know when they are merged.

Thanks again for your feedback.

@SekouDiaoNlp SekouDiaoNlp self-assigned this Jan 15, 2024
@SekouDiaoNlp SekouDiaoNlp added the bug Something isn't working label Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants