Repository navigation
fix: register nltk download path before downloading cmudict - #566
Merged
NeonDaniel merged 1 commit intoAug 7, 2026
Merged
Conversation
nltk 3.10 authorizes download targets against `nltk.data.path` (`nltk/pathsec.py`). `get_phonemes` appended its custom download directory after calling `nltk.download`, so the download was rejected with "Security Violation: Unauthorized path" and `cmudict` never installed. Register the path first. Verified against a cold cache: the existing `test_get_phonemes` fails before this change and passes after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NeonDaniel
approved these changes
Aug 7, 2026
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
The
unit_testsworkflow fails ondevat the Test Parse Utils step, on Python 3.10, 3.11, and 3.12. The failed test istests/parse_util_tests.py::ParseUtilTests::test_get_phonemes.The cause is a change in nltk 3.10, not a change in this repository. nltk added
nltk/pathsec.py, which authorizes a download target against the directories innltk.data.path.get_phonemes(neon_utils/parse_utils.py:115) registered its download directory one line after the download call:The download ran against a path that was not yet registered, so nltk refused it:
cmudictdid not install. The corpus lookup then raisedLookupError.requirements/requirements.txtpinsnltk~=3.5, which permits 3.10.2. The lastdevrun that passed was 2026-06-08, before this nltk release.Change
Register the download path before the download. The
intest prevents duplicate entries innltk.data.pathon repeat calls.Test plan
nltk.download('cmudict', download_dir=...)returnsFalsewith the security violation above when the directory is absent fromnltk.data.path. It returnsTruewhen the directory is present.test_get_phonemesfails before this change and passes after it.tests/parse_util_tests.py: 15 passed from a cold cache.Note
This change is independent of #565. #565 starts from the same commit and shows the same CI fault, which this change corrects.
🤖 Generated with Claude Code