Feat: Create InMemoryTarget from TaskOnKart#441
Open
utotsubasa wants to merge 19 commits intom3dev:masterfrom
Open
Feat: Create InMemoryTarget from TaskOnKart#441utotsubasa wants to merge 19 commits intom3dev:masterfrom
InMemoryTarget from TaskOnKart#441utotsubasa wants to merge 19 commits intom3dev:masterfrom
Conversation
feat: last_modification_time feature in `InMemoryTarget` style: add some type hints fix: fix typo in `InMemoryCacheRepository` test: add some tests for `InMemoryTarget` and `InMemoryCacheRepository`
style: update variable name from `id` to `key`
chore: add type hints style: remove `Protocol`
feat: add the new parameter `cache_in_memory_by_default` to switch default Target style: update the variable name from `target_key` to `data_key` for code consistency test: add tests for `TaskOnKart`s with the `cache_in_memory` parameter
utotsubasa
force-pushed
the
feat/in_memory_parameter_on_task
branch
from
fa0cbd3 to
76cb255
Compare
style: add a type hint
utotsubasa
changed the title
InMemoryTarget from TargetOnKart
utotsubasa
changed the title
InMemoryTarget from TargetOnKartInMemoryTarget from TaskOnKart
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the InMemoryTarget functionality to TaskOnKart by adding a new parameter (cache_in_memory_by_default) that allows tasks to store outputs in memory rather than on file by default. It also updates tests to confirm correct target creation and behavior and refactors target creation functions for better clarity.
- Added cache_in_memory_by_default parameter and associated methods in TaskOnKart.
- Updated test files to verify that cache targets and file targets are created as expected.
- Refactored make_in_memory_target in the InMemoryTarget module to support an optional unique id.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/in_memory/test_task_cached_in_memory.py | Added tests to verify in-memory caching behavior and target paths. |
| test/in_memory/test_in_memory_target.py | Updated fixture to use data_key to create in-memory targets. |
| gokart/task.py | Introduced cache_in_memory_by_default parameter and logic for default targets. |
| gokart/in_memory/target.py | Refactored make_in_memory_target; added helper _make_data_key. |
Comments suppressed due to low confidence (1)
gokart/in_memory/target.py:43
- Typo in the TODO comment: 'migit' should be corrected to 'might'.
# TODO: this module name `_path` migit not be appropriate
kitagry
reviewed
May 31, 2025
|
|
||
| def output(self) -> FlattenableItems[TargetOnKart]: | ||
| return self.make_target() | ||
| return self.make_default_target() |
Member
There was a problem hiding this comment.
I prefer to change here instead of overwriting a method.
Suggested change
| return self.make_default_target() | |
| return self.make_target if not self.cache_in_memory_by_default()else self.make_cache_target() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TBE