Skip to content

Commit

Permalink
final self review
Browse files Browse the repository at this point in the history
  • Loading branch information
PythonFZ committed Feb 18, 2022
1 parent 3efca81 commit c79c364
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 41 deletions.
13 changes: 0 additions & 13 deletions examples/docs/08_custom_zntrackoptions.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -240,19 +240,6 @@
"is_executing": true
}
}
},
{
"cell_type": "code",
"execution_count": null,
"outputs": [],
"source": [],
"metadata": {
"collapsed": false,
"pycharm": {
"name": "#%%\n",
"is_executing": true
}
}
}
],
"metadata": {
Expand Down
15 changes: 8 additions & 7 deletions examples/docs/09_lazy.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
{
"cell_type": "markdown",
"source": [
"We will now create a PrintOption that is identical to `zn.outs` put prints a message every time the data is read from files."
"We will now create a PrintOption that is identical to `zn.outs` but prints a message every time the data is read from files."
],
"metadata": {
"collapsed": false,
Expand Down Expand Up @@ -152,7 +152,7 @@
{
"cell_type": "markdown",
"source": [
"In this first Example we will won't use lazy loading."
"In this first Example we will not use lazy loading."
],
"metadata": {
"collapsed": false,
Expand Down Expand Up @@ -294,7 +294,7 @@
{
"cell_type": "markdown",
"source": [
"We can see, that the random number is not yet available but as soon as we access the attribute it will be loaded for us."
"We can see, that the random number is not yet available but as soon as we access the attribute it will be loaded for us (and stored in memory)."
],
"metadata": {
"collapsed": false,
Expand Down Expand Up @@ -336,7 +336,7 @@
{
"cell_type": "markdown",
"source": [
"Let's build some dependencies to show where this can become an issue."
"Let's build some dependencies to show where lazy loading is especially useful."
],
"metadata": {
"collapsed": false,
Expand Down Expand Up @@ -695,7 +695,7 @@
{
"cell_type": "markdown",
"source": [
"If we now load the latest AddOne we will see that it loads up everything into memory, although we might only be interested in the most recent number."
"If we now load the latest `AddOne` we will see that it loads up everything into memory, although we might only be interested in the most recent number."
],
"metadata": {
"collapsed": false,
Expand Down Expand Up @@ -766,7 +766,7 @@
{
"cell_type": "markdown",
"source": [
"We can check with an arbitrary depth of dependencies to ensure that both instances use the same values."
"We can check with an arbitrary depth of dependencies that both instances yield the same value."
],
"metadata": {
"collapsed": false,
Expand Down Expand Up @@ -832,7 +832,8 @@
"cell_type": "markdown",
"source": [
"## Notes\n",
"When using ZnTrack to compare data of different versions it is important to either not use `lazy=True` or load the data manually before loading another version of the data."
"When using ZnTrack to compare data of different versions it is important to either not use `lazy=True` or load the data manually before loading another version of the data.\n",
"In the following example we store the result of `dvc repro` for three different experiments with and without `lazy=True` and compare the results."
],
"metadata": {
"collapsed": false,
Expand Down
2 changes: 1 addition & 1 deletion zntrack/core/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def update_options(self, lazy=None):
# trigger loading the data into memory
value = getattr(self, option.name)
try:
value.update_options(lazy=lazy)
value.update_options(lazy=False)
except AttributeError:
# if lazy=False trigger update_options iteratively on
# all dependency Nodes
Expand Down
46 changes: 26 additions & 20 deletions zntrack/core/zntrackoption.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"""
from __future__ import annotations

import copy
import logging
import pathlib
import typing
Expand Down Expand Up @@ -90,32 +91,36 @@ def __str__(self):

def __get__(self, instance, owner):
self._instance = instance

if instance is None:
return self

if not instance.is_loaded:
elif instance.is_loaded:
is_lazy_option = instance.__dict__.get(self.name) is utils.LazyOption
is_not_in_dict = self.name not in instance.__dict__

if is_lazy_option or is_not_in_dict:
# the __dict__ only needs to be updated if __dict__ does
# not contain self.name or self.name is LazyOption
try:
# load data and store it in the instance
instance.__dict__[self.name] = self.get_data_from_files(instance)
log.debug(f"instance {instance} updated from file")
except (KeyError, FileNotFoundError, AttributeError) as err:
# do not load default value, because a loaded instance should always
# load from files.
if not utils.config.allow_empty_loading:
# allow overwriting this
raise AttributeError(
f"Could not load {self.name} for {instance}"
) from err
else:
# if the instance is not loaded, there is no LazyOption handling
try:
return instance.__dict__[self.name]
except KeyError:
instance.__dict__[self.name] = self.default_value

is_lazyoption = instance.__dict__.get(self.name) is utils.LazyOption
not_in_dict = self.name not in instance.__dict__

if (is_lazyoption or not_in_dict) and instance.is_loaded:
try:
# load data and store it in the instance
instance.__dict__[self.name] = self.get_data_from_files(instance)
log.debug(f"instance {instance} updated from file")
except (KeyError, FileNotFoundError, AttributeError) as err:
# do not load default value, because a loaded instance should always
# load from files.
if not utils.config.allow_empty_loading:
# allow overwriting this
raise AttributeError(
f"Could not load {self.name} for {instance}"
) from err
# instead of .get(name, default_value) we make a copy of the default value
# because it could be changed.
instance.__dict__[self.name] = copy.deepcopy(self.default_value)

return instance.__dict__[self.name]

Expand All @@ -136,6 +141,7 @@ def save(self, instance):
to be passed manually.
"""
if instance.__dict__.get(self.name) is utils.LazyOption:
# do not save anything if __get__/__set__ was never used
return
utils.file_io.update_config_file(
file=self.get_filename(instance),
Expand Down

0 comments on commit c79c364

Please sign in to comment.