Skip to content

Remove hardcoded registry file - #199

Merged
lfoppiano merged 2 commits into
masterfrom
bugfix/fix-hardcoded-resource-file
Mar 18, 2026
Merged

Remove hardcoded registry file #199
lfoppiano merged 2 commits into
masterfrom
bugfix/fix-hardcoded-resource-file

Conversation

@lfoppiano

Copy link
Copy Markdown
Collaborator

This pull request refactors how the resource registry is handled across the sequence labeling models in the delft package. The main change is to allow the registry path to be set via an environment variable and to enable passing a custom registry to model constructors, improving flexibility and configurability. All model classes and the model factory function (get_model) are updated to support this new approach.

Resource registry handling improvements:

  • Added a get_registry_path function to delft/__init__.py to allow the registry path to be configured via the DELFT_REGISTRY_PATH environment variable, falling back to the default location if not set.
  • Updated the BaseModel constructor in delft/sequenceLabelling/models.py to accept a registry parameter and load the registry using the new logic if none is provided.

Model instantiation changes:

  • Modified the get_model function and all model class constructors in delft/sequenceLabelling/models.py to accept and propagate the registry parameter, ensuring consistent use of the registry throughout the codebase.

@lfoppiano lfoppiano linked an issue Mar 4, 2026 that may be closed by this pull request
@lfoppiano
lfoppiano marked this pull request as draft March 4, 2026 15:31
Comment thread delft/textClassification/models.py
@lfoppiano

lfoppiano commented Mar 4, 2026

Copy link
Copy Markdown
Collaborator Author

Here one example of use, I created a new registry with a new db path:

(delft) (base) ➜  delft git:(bugfix/fix-hardcoded-resource-file) ✗ export DELFT_REGISTRY_PATH=resources-registry2.json 
(delft) (base) ➜  delft git:(bugfix/fix-hardcoded-resource-file) ✗ .venv/bin/python -m delft.applications.grobidTagger date train --architecture BidLSTM_CRF
Loading data...
Using latest training file: data/sequenceLabelling/grobid/date/date-220311.train
637 total sequences
573 train sequences
64 validation sequences

max train sequence length: 12
max validation sequence length: 10

Compiling embeddings... (this is done only one time per embeddings at first usage)
error: embedding path for glove-840B is not valid /media/lopez/T5/embeddings/glove.840B.300d.txt
Downloading resource file for glove-840B ...
downloading https://huggingface.co/stanfordnlp/glove/resolve/main/glove.840B.300d.zip
data/download/glove.840B.300d.zip:   1%|▍                                                                       | 12.5M/2.03G [00:10<37:40, 957kiB/s]
[...]

@lfoppiano
lfoppiano marked this pull request as ready for review March 4, 2026 19:59
@lfoppiano
lfoppiano requested a review from de-code March 4, 2026 19:59

@de-code de-code left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good.

Didn't quite expect the extend of necessary changes when I raised the issue.

@lfoppiano
lfoppiano merged commit 6ba8842 into master Mar 18, 2026
4 checks passed
@lfoppiano
lfoppiano deleted the bugfix/fix-hardcoded-resource-file branch March 18, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow overriding the resource registry

2 participants