Add "alternative_spellings" property + minor bug fix for words with alternative spellings in title #147
Conversation
This reverts commit 051d01b.
|
Thank you again for your pull request! Sorry for the late reply. Let me quickly review the code. |
There was a problem hiding this comment.
Please try running make check and looking at the issues reported.
autopep8 and flake8 reports some issues:
$ make check
...
flake8 --builtins="_" --max-line-length 99 duden/ tests/
duden/word.py:61:72: W291 trailing whitespace
duden/word.py:62:1: W293 blank line contains whitespace
duden/word.py:103:74: W291 trailing whitespace
duden/word.py:111:43: E203 whitespace before ','
duden/display.py:157:89: W292 no newline at end of file
Makefile
Outdated
| ./run_duden.py --export Keyboard > tests/test_data/Keyboard.yaml | ||
| ./run_duden.py --export Meme > tests/test_data/Meme.yaml | ||
|
|
There was a problem hiding this comment.
This is a triviality, there's an extra tab at the end of the file. Ideally, the file would end with a single newline.
duden/display.py
Outdated
| @@ -152,3 +152,6 @@ def describe_word(word): | |||
| for part_of_speech, words in word.compounds.items(): | |||
| print(blue(' - {}:'.format(part_of_speech.capitalize())), | |||
| ', '.join(words)) | |||
|
|
|||
| if word.alternative_spellings: | |||
| print(white(_('Alternative_spellings:'), bold=True), word.alternative_spellings) | |||
There was a problem hiding this comment.
This is supposed to be human-readable string, so 'Alternative spellings:' without underscore should be better. Also could you add a newline at the end of the file?
duden/word.py
Outdated
| article_element = self.soup.find('span', {"class": "lemma__determiner"}) | ||
| if article_element is not None: | ||
| # remove soft hyphens "\xad" and return | ||
| return article_element.get_text().replace('\xad', '').strip() |
There was a problem hiding this comment.
Reusing the clear_text function from common.py would reduce code duplication.
| return article_element.get_text().replace('\xad', '').strip() | |
| return clear_text(article_element.get_text()) |
radomirbosak#147 (comment) Co-authored-by: Radomír Bosák <radomir.bosak@gmail.com>
Co-authored-by: Radomír Bosák <radomir.bosak@gmail.com>
|
I apologize for the many small mistakes, it's my first time working with python and initially I didn't get the linter to work (on windows). Thank you for staying kind :) I think, this is it. If you want anything else changed, let me know! |
There was a problem hiding this comment.
That's perfect! Thank you for making these changes.
I apologize for the many small mistakes, it's my first time working with python and initially I didn't get the linter to work (on windows). Thank you for staying kind :)
No worries. I'm not exactly sure if the linters work on Windows or not since I have no way of testing it.
I added one more comment.
duden/word.py
Outdated
| article_element = self.soup.find('span', {"class": "lemma__determiner"}) | ||
| if article_element is not None: | ||
| # remove soft hyphens "\xad" and return | ||
| return article_element.get_text().replace('\xad', '').strip() |
There was a problem hiding this comment.
clear_text can be used here as well
|
duden 0.16.0 is out and available on pypi supporting to upgrade: |
#146