dash and lowered_split fix for inflect.py _plnoun() #127
Conversation
|
Can you add a test that captures the missed expectation prior to the patch? |
|
I regret not having responded to this sooner. It does now appear to be abandoned. Please feel free to revive the effort at some point. |
|
I'm guessing one would have to trigger this with 'prima-donna' then one would get back something like 'prima donnas' and the singular prima donna in the tests/test_pl_si.py when added to tests/words.txt. Maybe there wiyld be more if pl_sb_irregular_compound would contain more words in the future, like man-of-war, jack-in-the-pulpit What is the concern with this change? It seems to fix a dash- space confusion, are you inclined to think this was intentional? |
It's changing behavior in a function that's already unmanageably complex without any tests. That is, I could revert the change and the tests will still pass. To be sure, it's not your fault the implementation is so complex. Moreover, Man-of-war may be a better example. Still, I'd like to see some tests added that capture not only the missed expectations, but also expectations around proximate concerns (such as space-separated words or compound subjects with only one dash). |
|
Looking at this a bit more, I see that there's similar code in _sinoun that already been re-factored to handle spaces and dashes differently. It would be nice if there were a way to unify this logic too. |
|
See #153, where I've refactored the code to unify the logic around prepositional phrases. |
|
My mistake. I see now that #126 captures the issue. Let's continue the conversation there. |
In function _plnoun(), the variable lowered_split is split on dashes, but when reusing this variable, code required whitespace split lowerd words, not dash split.