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

Move etymology lookup to wiktionary #1178

Merged
merged 2 commits into from
May 21, 2020
Merged

Move etymology lookup to wiktionary #1178

merged 2 commits into from
May 21, 2020

Conversation

dgw
Copy link
Member

@dgw dgw commented Feb 20, 2017

This spelling error prevents etymology lines from matching, because the code sets the mode to etymology but then later looks for a mode equal to etmyology. Fixed.

@dgw dgw added this to the 6.5.2 milestone Mar 27, 2018
@dgw dgw added the Tweak label Mar 27, 2018
@dgw dgw self-assigned this Mar 27, 2018
@dgw
Copy link
Member Author

dgw commented Mar 27, 2018

This actually accomplishes nothing, because the returned etymology from wikt() is never used. Perhaps this PR will morph into a pseudo-fix for #1248 (moving the .ety command into wiktionary.py)?

@dgw dgw modified the milestones: 6.5.2, 6.6.0 Mar 27, 2018
@dgw dgw changed the title [wiktionary] Fix spelling mistake blocking etymology output Replace broken etymology module with wiktionary implementation May 6, 2018
@dgw
Copy link
Member Author

dgw commented May 6, 2018

Built a proof-of-concept replacement .ety command on this branch, since the original PR was useless.

The wiktionary module only parses definitions for the first sense of a word, and the new .ety command implementation only looks for the first etymology (if it exists) to match that. A future rewrite (either in this PR or another, not yet decided) will add handling of multiple senses to both commands.

The Coveralls check failure for -0.9% test coverage tells me that my rewrite should also add example tests to this module. 12% coverage is downright awful. (The large drop must be from removing etymology.py, which was technically "30% covered" just from defining functions & initializing a ton of global variables.)

(Future implementation notes) Good words for testing this future rewrite include "sake" and "forte". As of writing this comment, both include two separate senses with independent etymologies and their own sets of definitions. (Edit: TIL you can't use Markdown inside <details> blocks… Thanks, GitHub.)

@dgw dgw added Bug Things to squish; generally used for issues Feature Medium Priority and removed Tweak labels May 6, 2018
These are at the mercy of Wiktionary editors changing the page content,
but it doesn't look like the entry for "bailiwick" is edited much.
@dgw dgw mentioned this pull request Sep 3, 2018
@dgw dgw modified the milestones: 6.6.0, 7.0.0 Jan 3, 2019
@dgw
Copy link
Member Author

dgw commented Jan 3, 2019

6.6.0 will have #1432 to fix #1248, and this rewrite will maybe happen later—or not.

@dgw dgw added Bugfix Generally, PRs that reference (and fix) one or more issue(s) and removed Bug Things to squish; generally used for issues labels Jan 3, 2019
@dgw dgw changed the title Replace broken etymology module with wiktionary implementation Move etymology lookup to wiktionary Jan 13, 2019
@dgw dgw modified the milestones: 7.0.0, 7.1.0 Oct 27, 2019
@dgw
Copy link
Member Author

dgw commented Oct 27, 2019

Shelving this until 7.1, unless etymology breaks again between now and 7.0's release.

@dgw dgw removed Work In Progress Bugfix Generally, PRs that reference (and fix) one or more issue(s) labels May 8, 2020
@dgw dgw requested a review from a team May 8, 2020 14:03
@dgw
Copy link
Member Author

dgw commented May 8, 2020

I have no idea why I left this as WIP. It clearly works:

<dgw> ,ety bailiwick
<SopelTest> bailiwick: From bailie (“bailiff”) and wick (“dwelling”), from Old
            English wīc.

The main difference is obviously how detailed the etymologies are, but I think Wiktionary is a better bet over the long term. WT's editors will continue to flesh it out over time.

I have a conflicts-resolved merge already done locally (it's not that bad) for testing, so don't worry about GitHub showing that this isn't mergeable @ reviewers.

@dgw dgw merged commit 510f35d into sopel-irc:master May 21, 2020
@dgw dgw deleted the wiktionary-etymology branch May 21, 2020 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants