Fix tilde in dataset path

[DRosen766]

This PR corresponds to this issue: Improper Dataset Extraction when Tilde In path #295

Previously, if a tilde existed in the path passed to a dataset for saving/extraction, the dataset would not be able to recognize the location to extract after downloading. This is because the function os.path.expanduser was only run during extraction here

but nowhere during loading.

To fix this, I added a call to os.path.expanduser in the Dataset class's init

I also added a global variable to the dataset_utils test to make changing the test directory easier and made a call to the os.path.expanduser function so it can be assigned to a tilde path and still work.

All the tests work on my end locally, lmk if they fail anywhere else or if something is missing.

Also, in order to run CICD successfully, I had to update the pytorch requirements.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.49%. Comparing base (a4e8a45) to head (30d993b).
Report is 72 commits behind head on develop.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #296      +/-   ##
===========================================
+ Coverage    77.07%   77.49%   +0.42%     
===========================================
  Files           54       55       +1     
  Lines         3067     3231     +164     
===========================================
+ Hits          2364     2504     +140     
- Misses         703      727      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[biphasic]

such hardcoded paths seem extremely brittle, maybe it would be worth adding a separate test for a path that includes a tilde

[fabrizio-ottati]

But this is the same as hardcoding `/tmp` on an Unix system! It’s better to use pathlib to define this path. No idea how to make it Windows friendly. On 14 Dec 2024, at 22:49, Gregor Lenz ***@***.***> wrote: @biphasic commented on this pull request. In test/dataset_utils.py:

+TEST_LOCATION_ON_SYSTEM = "~/../../tmp"

+TEST_LOCATION_ON_SYSTEM = os.path.expanduser(TEST_LOCATION_ON_SYSTEM) such hardcoded paths seem extremely brittle, maybe it would be worth adding a separate test for a path that includes a tilde —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[DRosen766]

But this is the same as hardcoding /tmp on an Unix system! It’s better to use pathlib to define this path. No idea how to make it Windows friendly. On 14 Dec 2024, at 22:49, Gregor Lenz @.***> wrote:

@biphasic commented on this pull request.

In test/dataset_utils.py:

+TEST_LOCATION_ON_SYSTEM = "~/../../tmp"

+TEST_LOCATION_ON_SYSTEM = os.path.expanduser(TEST_LOCATION_ON_SYSTEM)

such hardcoded paths seem extremely brittle, maybe it would be worth adding a separate test for a path that includes a tilde

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

@fabrizio-ottati @biphasic
Yea that was my reasoning for leaving it as a hardcoded value. I just converted it to a global constant to avoid several magic constants with the same value.

I can parameterize it with various paths but since there is no parameterization I left it as one value.