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

[BUG] Example.py has wrong DATA_PATH #1703

Open
thethiny opened this issue Oct 22, 2024 · 7 comments
Open

[BUG] Example.py has wrong DATA_PATH #1703

thethiny opened this issue Oct 22, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@thethiny
Copy link

thethiny commented Oct 22, 2024

Describe the bug

DATA_PATH = Path(__file__).parent

The following line gets the parent of the __file__ which is incorrect when I'm importing the library. The file should be the current directory.

To Reproduce

from pytorch_forecasting.data.examples import get_stallion_data
get_stallion_data()

Versions

pytorch_forecast version 1.1.1

@thethiny thethiny added the bug Something isn't working label Oct 22, 2024
@fkiraly
Copy link
Collaborator

fkiraly commented Oct 22, 2024

Thanks for the report!

The fix seems simple, so a pull request with a fix, and a simple test would be much appreciated! It appears that this was not tested, then?

@thethiny
Copy link
Author

thethiny commented Oct 22, 2024

Seems like the issue is with the entire examples folder, as multiple files have not been updated to reflect newer changes, such as stallion.py complains about TensorBoard as save_dir is not specified, which has been necessary since pytroch-forecasting 1.1.1 (Was not an issue with 0.10.0 which is when the error started).

I'm doing some research and running the examples is necessary for me, therefore I will probably end up testing it. As for creating new tests, I will look into that as the testing necessary is through actions and not only through pytest.

@fkiraly
Copy link
Collaborator

fkiraly commented Oct 22, 2024

Thanks! Testing via pytest would be great, because those are run on every PR and regularly, hence very frequently.

I am surprised that the examples are not tested. We test the notebooks though, now, that was added in 1.1.

@thethiny
Copy link
Author

From what I've seen the testing happens over the notebooks as they're converted to .py files before testing.
As for using pytest I still don't believe it'll work since the issue occurs only when the library is installed and even then it won't throw an "error".

For the other errors I can manage with pytest.

@fkiraly
Copy link
Collaborator

fkiraly commented Oct 22, 2024

As for using pytest I still don't believe it'll work since the issue occurs only when the library is installed and even then it won't throw an "error".

Could you explain that, i.e., why? I do not get this. The pytorch-forecasting library is installed in all the testing environments, so why is it relevant that the issue occurs only when the library is installed?

@thethiny
Copy link
Author

thethiny commented Oct 23, 2024

Could you explain that, i.e., why? I do not get this. The pytorch-forecasting library is installed in all the testing environments, so why is it relevant that the issue occurs only when the library is installed?

Because when the repo is cloned, the clone path is relative to your working dir, and the data is downloaded relative to the clone dir. But when the library is installed, the path is absolute and not relative, it is under ~ in most cases, therefore the data is downloaded in that folder under the libs/site-packages folder.

@fkiraly
Copy link
Collaborator

fkiraly commented Oct 27, 2024

I see, let's change this so it works in both cases then. Looking forward to a PR, that would be appreciated!

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
Status: Reproduced/confirmed
Development

No branches or pull requests

2 participants