fix: WordNet lex_filename + adjective markers and FrameNet FE constraints#10
Merged
Merged
Conversation
…markers Two independent WordNet converter bugs surfaced while mapping WordNet into downstream consumers: - lex_filename was derived from a mis-ordered table: adv.all and adj.ppl were misplaced and every entry from index 2 onward was shifted by one, so e.g. lex_filenum=29 (verb.body) was labeled noun.time and lex_filenum=3 (noun.Tops) was labeled adv.all. Align LEX_FILE_NAMES with the canonical WordNet lexnames lexicographer-file table. - Adjective syntactic-position markers ((p)/(a)/(ip)) were left appended to the word token, which failed lemma validation and caused the entire synset to be silently dropped. Parse the marker off the word into the new Word.syntactic_marker field instead. Closes #9 Closes #8
requiresFE/excludesFE are self-closing siblings of <FE> whose target FE name lives in the name attribute. The converter used find() to grab the first such element and then iterated its (nonexistent) child elements, so requires_fe and excludes_fe were always empty. Use findall() and read the name attribute from each sibling. Closes #7
Add a twine check gate, link the pypi environment to the project URL, and create a matching GitHub Release (which also gives Zenodo a release to archive) after a successful publish. Publishing still uses OIDC trusted publishing gated by the pypi environment; no API tokens are stored.
mypy 2.x narrows dataset_lower to DatasetType after the membership guard, so the subsequent cast became a redundant-cast error under --strict. Fold the cast into the lookup on the still-unnarrowed str and use dict.get, which keeps the cast genuinely needed (so it passes mypy 1.x and 2.x) without changing behavior: unsupported names still raise ValueError.
The unpinned `mypy>=1.8.0` dev spec let CI resolve a newer mypy (2.x) than local environments, whose stricter `--strict` analysis failed Type Check on code that passed locally. Pin the exact version so CI and local runs agree; bump deliberately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all three open issues, found while mapping these resources into downstream consumers. Each is an independent converter-correctness bug with new regression tests.
fix(wordnet)—lex_filename↔lex_filenummismatch (#9)LEX_FILE_NAMESwas mis-ordered:adv.all/adj.pplwere misplaced and every entry from index 2 onward was shifted by one. Solex_filenum=29(canonicallyverb.body) was labelednoun.time, andlex_filenum=3(noun.Tops) was labeledadv.all. The table is now aligned to the canonical WordNetlexnameslexicographer-file table.fix(wordnet)— adjective syntactic-position markers (#8)WordNet appends
(p)/(a)/(ip)markers directly onto adjective word tokens (e.g.galore(ip)). The marker stayed on the lemma, failed lemma validation, and the entire synset was silently dropped during conversion — so this was data loss, not just a missing field. The marker is now parsed off into a new optionalWord.syntactic_markerfield (additive, backward-compatible), and the synset is retained.fix(framenet)—requiresFE/excludesFEempty (#7)requiresFE/excludesFEare self-closing siblings of<FE>whose target FE name lives in thenameattribute. The converter usedfind()to grab the first such element and then iterated its (nonexistent) child elements, sorequires_fe/excludes_fewere always empty (0 of 11,428 FEs). It now usesfindall()and reads thenameattribute from each sibling. Verified against the real FN 1.7Personal_relationshipframe.ci(release)— harden the PyPI release workflowPer request: added a
twine checkgate, linked thepypienvironment to the project URL, and added a GitHub Release step (which also gives Zenodo a release to archive) after a successful publish. Still OIDC trusted publishing gated by thepypienvironment — no API tokens.Version
Patch bump
0.2.2 → 0.2.3(CHANGELOG + docs updated). The only API change is the additive optionalWord.syntactic_markerfield.Testing
Full CI suite run locally and green:
ruff check,ruff format --check,mypy --strict src/(no ignores added),pytest(1395 passed), clean-venv minimal install, integration CLI + sample conversion, andpython -m build+twine check. New tests cover all three fixes, including regression cases for the previously-dropped marker synsets and the canonicallexnamestable.Closes #7
Closes #8
Closes #9