search

sktime / pytorch-forecasting

Time series forecasting with PyTorch
https://pytorch-forecasting.readthedocs.io/
MIT License
3.97k stars 629 forks source link

[BUG] Example.py has wrong DATA_PATH #1703

Open thethiny opened 6 days ago

thethiny commented 6 days ago

Describe the bug

https://github.com/sktime/pytorch-forecasting/blob/b8c39808bd1d7a0a8024e331d8af14c30d8f62a4/pytorch_forecasting/data/examples.py#L13

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

fkiraly commented 6 days ago

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 commented 6 days ago

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 commented 6 days ago

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 commented 6 days ago

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 commented 6 days ago

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 commented 5 days ago

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 commented 1 day ago

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