Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes the dependency on the torchtext library by transplanting its vocabulary functionality into the local codebase under lighthouse/common/vocab. The change eliminates an external dependency while maintaining the same vocabulary functionality needed for text processing.
- Replaces
torchtext.vocabimports with locallighthouse.common.vocabmodule - Creates a complete local implementation of vocabulary classes (
Vocab,GloVe,FastText, etc.) - Updates installation instructions and CI workflows to remove torchtext dependency
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| training/dataset.py | Updates import to use local vocab module instead of torchtext |
| training/cg_detr_dataset.py | Updates import to use local vocab module instead of torchtext |
| lighthouse/feature_extractor/text_encoders/glove.py | Updates import to use local vocab module instead of torchtext |
| lighthouse/common/vocab/vocab.py | Implements Vocab class with torch.jit compatibility |
| lighthouse/common/vocab/vectors.py | Implements vector classes (GloVe, FastText, CharNGram) for embeddings |
| lighthouse/common/vocab/init.py | Package initialization exposing vocab classes |
| lighthouse/common/vocab.py | Complete vocabulary implementation with all classes |
| mypy.ini | Removes torchtext from mypy ignore list |
| README.md | Removes torchtext from installation instructions |
| .github/workflows/pytest.yml | Removes torchtext from CI dependencies |
| .github/workflows/mypy_ruff.yml | Removes torchtext from CI dependencies |
lighthouse/common/vocab.py
Outdated
Comment on lines
13
to
14
| from .utils import reporthook | ||
|
|
There was a problem hiding this comment.
Import error: The module '.utils' is being imported but there's no evidence of a utils.py file in the lighthouse/common directory. The reporthook function is defined within the same file at line 263, making this import unnecessary and likely to cause a runtime error.
Suggested change
| from .utils import reporthook |
Comment on lines
+290
to
+291
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | ||
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), |
There was a problem hiding this comment.
Type mismatch: The dim parameter is passed as a string "300" but the GloVe constructor expects an integer. This should be dim=300 without quotes.
Suggested change
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | |
| "glove.42B.300d": partial(GloVe, name="42B", dim=300), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim=300), |
Comment on lines
+290
to
+291
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | ||
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), |
There was a problem hiding this comment.
Type mismatch: The dim parameter is passed as a string "300" but the GloVe constructor expects an integer. This should be dim=300 without quotes.
Suggested change
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | |
| "glove.42B.300d": partial(GloVe, name="42B", dim=300), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim=300), |
| "fasttext.simple.300d": partial(FastText, language="simple"), | ||
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | ||
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | ||
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), |
There was a problem hiding this comment.
Type mismatch: The dim parameter is passed as a string "25" but the GloVe constructor expects an integer. This should be dim=25 without quotes.
Suggested change
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim=25), |
Comment on lines
+290
to
+299
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | ||
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | ||
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | ||
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | ||
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | ||
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | ||
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | ||
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | ||
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | ||
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), |
There was a problem hiding this comment.
Type mismatch: The dim parameter is passed as a string "50" but the GloVe constructor expects an integer. This should be dim=50 without quotes.
Suggested change
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), | |
| "glove.42B.300d": partial(GloVe, name="42B", dim=300), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim=300), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim=25), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim=50), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim=100), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim=200), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim=50), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim=100), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim=200), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim=300), |
lighthouse/common/vocab.py
Outdated
Comment on lines
554
to
563
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | ||
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | ||
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | ||
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | ||
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | ||
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | ||
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | ||
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | ||
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | ||
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), |
There was a problem hiding this comment.
Type mismatch: The dim parameter is passed as a string "50" but the GloVe constructor expects an integer. This should be dim=50 without quotes.
Suggested change
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), | |
| "glove.42B.300d": partial(GloVe, name="42B", dim=300), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim=300), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim=25), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim=50), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim=100), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim=200), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim=50), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim=100), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim=200), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim=300), |
lighthouse/common/vocab.py
Outdated
Comment on lines
554
to
563
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | ||
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | ||
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | ||
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | ||
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | ||
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | ||
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | ||
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | ||
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | ||
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), |
There was a problem hiding this comment.
Type mismatch: The dim parameter is passed as a string "100" but the GloVe constructor expects an integer. This should be dim=100 without quotes.
Suggested change
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), | |
| "glove.42B.300d": partial(GloVe, name="42B", dim=300), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim=300), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim=25), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim=50), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim=100), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim=200), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim=50), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim=100), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim=200), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim=300), |
lighthouse/common/vocab.py
Outdated
Comment on lines
554
to
563
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | ||
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | ||
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | ||
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | ||
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | ||
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | ||
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | ||
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | ||
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | ||
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), |
There was a problem hiding this comment.
Type mismatch: The dim parameter is passed as a string "200" but the GloVe constructor expects an integer. This should be dim=200 without quotes.
Suggested change
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), | |
| "glove.42B.300d": partial(GloVe, name="42B", dim=300), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim=300), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim=25), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim=50), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim=100), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim=200), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim=50), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim=100), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim=200), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim=300), |
lighthouse/common/vocab.py
Outdated
Comment on lines
554
to
563
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | ||
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | ||
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | ||
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | ||
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | ||
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | ||
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | ||
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | ||
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | ||
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), |
There was a problem hiding this comment.
Type mismatch: The dim parameter is passed as a string "300" but the GloVe constructor expects an integer. This should be dim=300 without quotes.
Suggested change
| "glove.42B.300d": partial(GloVe, name="42B", dim="300"), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim="300"), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim="25"), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim="50"), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim="100"), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim="200"), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim="50"), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim="100"), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim="200"), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim="300"), | |
| "glove.42B.300d": partial(GloVe, name="42B", dim=300), | |
| "glove.840B.300d": partial(GloVe, name="840B", dim=300), | |
| "glove.twitter.27B.25d": partial(GloVe, name="twitter.27B", dim=25), | |
| "glove.twitter.27B.50d": partial(GloVe, name="twitter.27B", dim=50), | |
| "glove.twitter.27B.100d": partial(GloVe, name="twitter.27B", dim=100), | |
| "glove.twitter.27B.200d": partial(GloVe, name="twitter.27B", dim=200), | |
| "glove.6B.50d": partial(GloVe, name="6B", dim=50), | |
| "glove.6B.100d": partial(GloVe, name="6B", dim=100), | |
| "glove.6B.200d": partial(GloVe, name="6B", dim=200), | |
| "glove.6B.300d": partial(GloVe, name="6B", dim=300), |
lighthouse/common/vocab.py
Outdated
| Examples: | ||
| >>> examples = ['chip', 'baby', 'Beautiful'] | ||
| >>> vec = text.vocab.GloVe(name='6B', dim=50) | ||
| >>> ret = vec.get_vecs_by_tokens(tokens, lower_case_backup=True) |
There was a problem hiding this comment.
Variable name inconsistency in docstring: The parameter name is 'examples' in the previous line but 'tokens' is used here. Should be 'ret = vec.get_vecs_by_tokens(examples, lower_case_backup=True)' for consistency.
Suggested change
| >>> ret = vec.get_vecs_by_tokens(tokens, lower_case_backup=True) | |
| >>> ret = vec.get_vecs_by_tokens(examples, lower_case_backup=True) |
Contributor
Author
|
Suggestions by copilot are trivial and I ignore them. |
Contributor
|
@h-munakata LGTM. |
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
2 participants
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.
This PR is related to #68 .
I transplanted torchtext's vocab into
lighthouse/common/vocab.This change effects to the following scripts:
lighthouse/feature_extractor/text_encoders/glove.pytraining/dataset.pytraining/cg_detr_dataset.pyI confirmed that training runs correctly.