update docstring and eval with overrides #6241
Conversation
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
wyli
force-pushed
the
resolves-lazy-eval
branch
from
e518132 to
304eb05
Compare
|
@atbenmurray is planning on making some changes to a number of aspects this PR touches so perhaps we wait on this one a bit. |
|
Still need a pause on this one for the moment |
| ) | ||
| if first_random is not None: | ||
| item_transformed = self.transform(item_transformed, start=first_random) | ||
| item_transformed = self.transform(item_transformed, start=first_random) |
There was a problem hiding this comment.
execute shouldn't have to handle start is None I think
| end_ = len(transforms) if end is None else end | ||
| if start is None: | ||
| raise ValueError(f"'start' ({start}) cannot be None") | ||
| input_ = evaluate_with_overrides( |
There was a problem hiding this comment.
I am not sure this is the correct way to handle this issue. I know you are trying to isolate dataset from changes, but I don't understand why evaluate_with_overrides needs to be called
There was a problem hiding this comment.
In any case, please hold off on this for now until I have assembled the PR that refactors some of this code for simplicity.
There was a problem hiding this comment.
mainly to replicate this line on the dev branch:
Line 355 in be3d138
so that the final transform always gets the pending operations evaluated
There was a problem hiding this comment.
If first_random is None, that means your entire compose was run before the item was cached, which would include the final evaluation. When the item is loaded from the cache, it has no pending transforms, so you have no way for pending transforms to be added that would then need to be evaluated
|
As per the review on #6244, if we agree that we aren't supporting caching of pending transforms, we can close this PR without a merge. |
3 participants
Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.