search

Safe-DS / Runner

Execute Safe-DS programs that were compiled to Python.
MIT License
2 stars 0 forks source link

feat: support relative paths #83

Closed lars-reimann closed 6 months ago

lars-reimann commented 6 months ago

Closes #76

Summary of Changes

The cwd of the worker process can now be set via the new data.cwd item of the program message.

github-actions[bot] commented 6 months ago

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 5 0 0 0.88s
✅ PYTHON mypy 5 0 2.65s
✅ PYTHON ruff 5 0 0 0.04s
✅ REPOSITORY git_diff yes no 0.01s

See detailed report in MegaLinter reports _Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff_

_MegaLinter is graciously provided by OX Security_

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (c32a4b6) to head (5dae678).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #83 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 11 11 Lines 522 530 +8 ========================================= + Hits 522 530 +8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lars-reimann commented 6 months ago

The code is fine, just one conceptual question: How does this work with memoization?

Changing the cwd could lead to the same (relative) path resolving to different files. As the timestamp is used to verify the valid re-usage of a file, this would probably not break correctness (and should be reasonably efficient, since the "older" files are not evicted from the cache (yet)), but it might cause confusion.

Could file_mtime return a tuple containing the absolute path of the file and the mtime? The function would need to be renamed, though (e.g. file_id).

WinPlay02 commented 6 months ago

Could file_mtime return a tuple containing the absolute path of the file and the mtime? The function would need to be renamed, though (e.g. file_id).

That should work, although reusing cached values between calls containing an absolute path and calls containing a relative path will not work.

lars-reimann commented 6 months ago

Another idea would be changing the code generator to create

safeds_runner.memoized_static_call("...", Table.from_csv_file, [safeds_runner.path('titanic.csv')], [safeds_runner.file_mtime('titanic.csv')]

instead of

safeds_runner.memoized_static_call("..", Table.from_csv_file, ['titanic.csv'], [safeds_runner.file_mtime('titanic.csv')]
WinPlay02 commented 6 months ago

Another idea would be changing the code generator to create

safeds_runner.memoized_static_call("...", Table.from_csv_file, [safeds_runner.absolute_path('titanic.csv')], [safeds_runner.file_mtime('titanic.csv')]

instead of

safeds_runner.memoized_static_call("..", Table.from_csv_file, ['titanic.csv'], [safeds_runner.file_mtime('titanic.csv')]

The information about the parameters is available in the stubs via impurity information, so it could easily be added. That would be the cleanest solution, as it shouldn't have any drawbacks.

lars-reimann commented 6 months ago

I'll do that after lunch.

lars-reimann commented 6 months ago

:tada: This PR is included in version 0.10.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: