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

Issue with cache root #2207

Closed
Narsil opened this issue Apr 13, 2021 · 3 comments
Closed

Issue with cache root #2207

Narsil opened this issue Apr 13, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@Narsil
Copy link

Narsil commented Apr 13, 2021

Describe the bug
A clear and concise description of what the bug is.

It seems the cache root is wrong if it is defined in the environment instead of the default

To Reproduce
Steps to reproduce the behavior (e.g. which model did you train? what parameters did you use? etc.).

Create test.py:

from flair.data import Sentence
from flair.models import SequenceTagger

# make a sentence
sentence = Sentence('I love Berlin .')

# load the NER tagger
tagger = SequenceTagger.load('ner')

# run NER over sentence
tagger.predict(sentence)

and run
FLAIR_CACHE_ROOT=$HOME/.cache/ python test.py

Expected behavior
A clear and concise description of what you expected to happen.

Example should work out of th box

Screenshots
If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

  • OS [e.g. iOS, Linux]: ubuntu
  • Version [e.g. flair-0.3.2]: 0.8.1 (or master)

Additional context
Add any other context about the problem here.

Simple fix is to cast to Path too when coming from env.

@Narsil Narsil added the bug Something isn't working label Apr 13, 2021
@HallerPatrick
Copy link
Contributor

HallerPatrick commented Apr 21, 2021

I also looked into this and can reproduce the bug. In this context I searched every occurrence of flair.cache_root, which holds the env value. And every time it is used to construct a Path object.

What I therefore would propose is to initially construct the Path object in the flair/__init__.py and remove every other construction of it in the source code.

Otherwise only a Path construction in flair/models/sequence_tagger.py has to be added.

I could do a PR for both.

Note: There are 237 occurrences of Path(flair.cache_root)

@alanakbik
Copy link
Collaborator

@HallerPatrick thanks for looking into this - a PR would be great! I guess it would be easier to construct it once in the init.

@HallerPatrick
Copy link
Contributor

This can be closed :)

@Narsil Narsil closed this as completed May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants