From d78f1153560fbd81e5423231470e1b104e3bf482 Mon Sep 17 00:00:00 2001 From: Kluk Date: Wed, 19 Oct 2022 09:18:11 +0200 Subject: [PATCH 01/26] NPT-12400: append and extend --- src/neptune/new/attributes/namespace.py | 16 ++ src/neptune/new/exceptions.py | 5 + src/neptune/new/handler.py | 117 +++++++++++++++ src/neptune/new/internal/utils/__init__.py | 4 + tests/e2e/standard/test_series.py | 95 ++++++++++++ tests/unit/neptune/new/test_handler.py | 167 ++++++++++++++++++++- 6 files changed, 401 insertions(+), 3 deletions(-) diff --git a/src/neptune/new/attributes/namespace.py b/src/neptune/new/attributes/namespace.py index 1e6c93f54..80734d107 100644 --- a/src/neptune/new/attributes/namespace.py +++ b/src/neptune/new/attributes/namespace.py @@ -18,13 +18,16 @@ TYPE_CHECKING, Any, Dict, + Iterable, Iterator, List, Mapping, + Optional, Union, ) from neptune.new.attributes.attribute import Attribute +from neptune.new.attributes.series.series import Data from neptune.new.internal.container_structure import ContainerStructure from neptune.new.internal.utils.generic_attribute_mapper import ( NoValue, @@ -63,6 +66,19 @@ def __len__(self) -> int: def __iter__(self) -> Iterator[str]: yield from self._attributes.__iter__() + def log( + self, + value: Union[Data, Iterable[Data]], + step: Optional[float] = None, + timestamp: Optional[float] = None, + wait: bool = False, + **kwargs, + ) -> None: + if not isinstance(value, NamespaceVal): + value = NamespaceVal(value) + for k, v in value.value.items(): + self._container[f"{self._str_path}/{k}"].log(v, step, timestamp, wait, **kwargs) + def to_dict(self) -> Dict[str, Any]: result = {} for key, value in self._attributes.items(): diff --git a/src/neptune/new/exceptions.py b/src/neptune/new/exceptions.py index d4f3b2983..f2a552aed 100644 --- a/src/neptune/new/exceptions.py +++ b/src/neptune/new/exceptions.py @@ -1354,3 +1354,8 @@ def __init__(self): {correct}Need help?{end}-> https://docs.neptune.ai/getting_help """ super().__init__(message.format(**STYLES)) + + +class NeptuneUserApiInputException(NeptuneException): + def __init__(self, message): + super().__init__(message) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 4629099a9..9f2091884 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -16,6 +16,8 @@ from functools import wraps from typing import ( TYPE_CHECKING, + Any, + Dict, Iterable, List, Optional, @@ -38,14 +40,17 @@ from neptune.new.exceptions import ( MissingFieldException, NeptuneCannotChangeStageManually, + NeptuneUserApiInputException, ) from neptune.new.internal.artifacts.types import ArtifactFileData from neptune.new.internal.utils import ( is_collection, + is_dict_like, is_float, is_float_like, is_string, is_string_like, + iterable_size, verify_collection_type, verify_type, ) @@ -287,6 +292,118 @@ def log( self._container.set_attribute(self._path, attr) attr.log(value, step=step, timestamp=timestamp, wait=wait, **kwargs) + def append( + self, + value: Union[dict, Any], + step: Optional[float] = None, + timestamp: Optional[float] = None, + wait: bool = False, + **kwargs, + ) -> None: + """ + todo: NPT-12530 + """ + verify_type("step", step, (int, float, type(None))) + verify_type("timestamp", timestamp, (int, float, type(None))) + if is_collection(value): + raise NeptuneUserApiInputException("Value cannot be a collection") + if is_dict_like(value): + for element in value.values(): + if is_collection(element): + raise NeptuneUserApiInputException("Dict value cannot be a collection, use extend method") + with self._container.lock(): + attr = self._container.get_attribute(self._path) + if not attr: + attr = self._create_new_attribute(value) + self._container.set_attribute(self._path, attr) + attr.log(value, step=step, timestamp=timestamp, wait=wait, **kwargs) + + def _create_new_attribute(self, value): + if is_float(value): + return FloatSeries(self._container, parse_path(self._path)) + elif is_dict_like(value): + return Namespace(self._container, parse_path(self._path)) + elif is_string(value): + return StringSeries(self._container, parse_path(self._path)) + elif FileVal.is_convertable(value): + return FileSeries(self._container, parse_path(self._path)) + elif is_float_like(value): + return FloatSeries(self._container, parse_path(self._path)) + elif is_string_like(value): + self._warn_casting_to_string() + return StringSeries(self._container, parse_path(self._path)) + else: + raise NeptuneUserApiInputException("Value of unsupported type {}".format(type(value))) + + def _warn_casting_to_string(self): + warn_once( + message="The object you're logging will be implicitly cast to a string." + " We'll end support of this behavior in `neptune-client==1.0.0`." + " To log the object as a string, use `.log(str(object))` instead.", + stack_level=3, + ) + + def extend( + self, + values: Union[Dict[str, Iterable[Any]], Iterable[Any]], + steps: Optional[Iterable[float]] = None, + timestamps: Optional[Iterable[float]] = None, + wait: bool = False, + **kwargs, + ) -> None: + """ + todo: NPT-12530 + """ + if is_dict_like(values): + self._validate_dict_for_extend(values, steps, timestamps) + for dict_key in values.keys(): + values_size = iterable_size(values[dict_key]) + new_steps = steps if steps is not None else [None] * values_size + new_timestamps = timestamps if timestamps is not None else [None] * values_size + for (value, step, timestamp) in zip(values[dict_key], new_steps, new_timestamps): + self.append({dict_key: value}, step, timestamp, wait, **kwargs) + elif is_collection(values): + values_size = iterable_size(values) + self._validate_iterable_for_extend(values_size, steps, timestamps) + new_steps = steps if steps is not None else [None] * values_size + new_timestamps = timestamps if timestamps is not None else [None] * values_size + for (value, step, timestamp) in zip(values, new_steps, new_timestamps): + if is_collection(value) or is_dict_like(value): + self.append(str(value), step, timestamp, wait, **kwargs) + else: + self.append(value, step, timestamp, wait, **kwargs) + else: + raise NeptuneUserApiInputException( + "Value cannot be a collection." + " To append multiple values at once, use extend() instead:" + " run['series'].extend(collection)" + ) + + def _validate_iterable_for_extend( + self, values_size: int, steps: Optional[Iterable[float]] = None, timestamps: Optional[Iterable[float]] = None + ) -> None: + if steps is not None and iterable_size(steps) != values_size: + raise NeptuneUserApiInputException("Number of steps must be equal to number of values") + if timestamps is not None and iterable_size(timestamps) != values_size: + raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values") + + def _validate_dict_for_extend( + self, + values: Union[Dict[str, Iterable[Any]], Iterable[Any]], + steps: Optional[Iterable[float]] = None, + timestamps: Optional[Iterable[float]] = None, + ) -> None: + timestamps_size = None if timestamps is None else iterable_size(timestamps) + steps_size = None if steps is None else iterable_size(steps) + for dict_key in values: + if not is_collection(values[dict_key]): + raise NeptuneUserApiInputException("If value is a dict, all values in dict should be a collection") + dict_iterables_size = iterable_size(values[dict_key]) + if timestamps_size is not None and timestamps_size != dict_iterables_size: + raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values in dict") + if steps_size is not None and steps_size != dict_iterables_size: + raise NeptuneUserApiInputException("Number of steps must be equal to number of values in dict") + @check_protected_paths def add(self, values: Union[str, Iterable[str]], wait: bool = False) -> None: """Adds the provided tag or tags to the run's tags. diff --git a/src/neptune/new/internal/utils/__init__.py b/src/neptune/new/internal/utils/__init__.py index a5959d320..2f543e08e 100644 --- a/src/neptune/new/internal/utils/__init__.py +++ b/src/neptune/new/internal/utils/__init__.py @@ -86,6 +86,10 @@ def is_dict_like(var): return isinstance(var, (dict, Mapping)) +def iterable_size(var): + return len(var) + + def is_string_like(var): try: _ = str(var) diff --git a/tests/e2e/standard/test_series.py b/tests/e2e/standard/test_series.py index ecad56f5b..59cf4b173 100644 --- a/tests/e2e/standard/test_series.py +++ b/tests/e2e/standard/test_series.py @@ -80,3 +80,98 @@ def test_log_images(self, container: MetadataContainer): for i in range(4): with Image.open(f"all/{i}.png") as img: assert img == image_to_png(image=images[i]) + + @pytest.mark.parametrize("container", AVAILABLE_CONTAINERS, indirect=True) + def test_append_numbers(self, container: MetadataContainer): + key = self.gen_key() + values = [random.random() for _ in range(50)] + for value in values: + container[key].append(value) + container.sync() + + assert container[key].fetch_last() == values[-1] + + fetched_values = container[key].fetch_values() + assert list(fetched_values["value"]) == values + + @pytest.mark.parametrize("container", AVAILABLE_CONTAINERS, indirect=True) + def test_append_strings(self, container: MetadataContainer): + key = self.gen_key() + values = [fake.word() for _ in range(50)] + for value in values: + container[key].append(value) + container.sync() + + assert container[key].fetch_last() == values[-1] + + fetched_values = container[key].fetch_values() + assert list(fetched_values["value"]) == values + + @pytest.mark.parametrize("container", AVAILABLE_CONTAINERS, indirect=True) + def test_append_images(self, container: MetadataContainer): + key = self.gen_key() + # images with size between 200KB - 12MB + images = list(generate_image(size=2**n) for n in range(8, 12)) + for value in images: + container[key].append(value) + container.sync() + + with tmp_context(): + container[key].download_last("last") + container[key].download("all") + + with Image.open("last/3.png") as img: + assert img == image_to_png(image=images[-1]) + + for i in range(4): + with Image.open(f"all/{i}.png") as img: + assert img == image_to_png(image=images[i]) + + @pytest.mark.parametrize("container", AVAILABLE_CONTAINERS, indirect=True) + def test_extend_numbers(self, container: MetadataContainer): + key = self.gen_key() + values = [random.random() for _ in range(50)] + + container[key].extend([values[0]]) + container[key].extend(values[1:]) + container.sync() + + assert container[key].fetch_last() == values[-1] + + fetched_values = container[key].fetch_values() + assert list(fetched_values["value"]) == values + + @pytest.mark.parametrize("container", AVAILABLE_CONTAINERS, indirect=True) + def test_extend_strings(self, container: MetadataContainer): + key = self.gen_key() + values = [fake.word() for _ in range(50)] + + container[key].extend([values[0]]) + container[key].extend(values[1:]) + container.sync() + + assert container[key].fetch_last() == values[-1] + + fetched_values = container[key].fetch_values() + assert list(fetched_values["value"]) == values + + @pytest.mark.parametrize("container", AVAILABLE_CONTAINERS, indirect=True) + def test_extend_images(self, container: MetadataContainer): + key = self.gen_key() + # images with size between 200KB - 12MB + images = list(generate_image(size=2**n) for n in range(8, 12)) + + container[key].extend([images[0]]) + container[key].extend(images[1:]) + container.sync() + + with tmp_context(): + container[key].download_last("last") + container[key].download("all") + + with Image.open("last/3.png") as img: + assert img == image_to_png(image=images[-1]) + + for i in range(4): + with Image.open(f"all/{i}.png") as img: + assert img == image_to_png(image=images[i]) diff --git a/tests/unit/neptune/new/test_handler.py b/tests/unit/neptune/new/test_handler.py index a817a232c..55debbae2 100644 --- a/tests/unit/neptune/new/test_handler.py +++ b/tests/unit/neptune/new/test_handler.py @@ -46,7 +46,10 @@ API_TOKEN_ENV_NAME, PROJECT_ENV_NAME, ) -from neptune.new.exceptions import FileNotFound +from neptune.new.exceptions import ( + FileNotFound, + NeptuneUserApiInputException, +) from neptune.new.types import File as FileVal from neptune.new.types.atoms.artifact import Artifact from neptune.new.types.atoms.datetime import Datetime as DatetimeVal @@ -212,6 +215,52 @@ def test_log(self): self.assertEqual(exp["some"]["str"]["val"].fetch_last(), "some text") self.assertIsInstance(exp.get_structure()["some"]["img"]["val"], FileSeries) + def test_log_dict(self): + exp = init(mode="debug", flush_period=0.5) + dict_value = {"key-a": "value-a", "key-b": "value-b"} + exp["some/num/val"].log(dict_value) + self.assertEqual(exp["some"]["num"]["val"].fetch_last(), str(dict_value)) + + def test_log_complex_input(self): + exp = init(mode="debug", flush_period=0.5) + exp["train/listOfDicts"].log([{"1a": 1, "1b": 1}, {"1a": 2, "1b": 2}]) + exp["train/listOfListsOfDicts"].log( + [[{"2a": 11, "2b": 11}, {"2a": 12, "2b": 12}], [{"2a": 21, "2b": 21}, {"2a": 22, "2b": 22}]] + ) + + self.assertEqual(exp["train"]["listOfDicts"].fetch_last(), "{'1a': 2, '1b': 2}") + self.assertEqual( + exp["train"]["listOfListsOfDicts"].fetch_last(), "[{'2a': 21, '2b': 21}, {'2a': 22, '2b': 22}]" + ) + + def test_append(self): + exp = init(mode="debug", flush_period=0.5) + exp["some/num/val"].append(5) + exp["some/str/val"].append("some text") + exp["some/img/val"].append(FileVal.as_image(PIL.Image.new("RGB", (60, 30), color="red"))) + exp["some/img/val"].append(PIL.Image.new("RGB", (60, 30), color="red")) + self.assertEqual(exp["some"]["num"]["val"].fetch_last(), 5) + self.assertEqual(exp["some"]["str"]["val"].fetch_last(), "some text") + self.assertIsInstance(exp.get_structure()["some"]["img"]["val"], FileSeries) + + def test_append_dict(self): + exp = init(mode="debug", flush_period=0.5) + dict_value = {"key-a": "value-a", "key-b": "value-b"} + exp["some/num/val"].append(dict_value) + self.assertEqual(exp["some"]["num"]["val"]["key-a"].fetch_last(), "value-a") + self.assertEqual(exp["some"]["num"]["val"]["key-b"].fetch_last(), "value-b") + + def test_append_complex_input(self): + exp = init(mode="debug", flush_period=0.5) + exp["train/dictOfDicts"].append( + { + "key-a": {"aa": 11, "ab": 22}, + "key-b": {"ba": 33, "bb": 44}, + } + ) + self.assertEqual(exp["train"]["dictOfDicts"]["key-a"].fetch_last(), "{'aa': 11, 'ab': 22}") + self.assertEqual(exp["train"]["dictOfDicts"]["key-b"].fetch_last(), "{'ba': 33, 'bb': 44}") + def test_log_many_values(self): exp = init(mode="debug", flush_period=0.5) exp["some/num/val"].log([5, 10, 15]) @@ -226,6 +275,65 @@ def test_log_many_values(self): self.assertEqual(exp["some"]["str"]["val"].fetch_last(), "other") self.assertIsInstance(exp.get_structure()["some"]["img"]["val"], FileSeries) + def test_append_many_values_cause_error(self): + exp = init(mode="debug", flush_period=0.5) + with self.assertRaises(NeptuneUserApiInputException): + exp["some/num/val"].append([]) + with self.assertRaises(NeptuneUserApiInputException): + exp["some/num/val"].append([5, 10, 15]) + with self.assertRaises(NeptuneUserApiInputException): + exp["some/str/val"].append(["some text", "other"]) + with self.assertRaises(NeptuneUserApiInputException): + exp["some/num/val"].append({"key-a": [1, 2]}) + with self.assertRaises(NeptuneUserApiInputException): + exp["some/img/val"].append( + [ + FileVal.as_image(PIL.Image.new("RGB", (60, 30), color="red")), + FileVal.as_image(PIL.Image.new("RGB", (20, 90), color="red")), + ] + ) + + def test_extend(self): + exp = init(mode="debug", flush_period=0.5) + exp["some/num/val"].extend([5, 7]) + exp["some/str/val"].extend(["some", "text"]) + exp["some/img/val"].extend( + [ + FileVal.as_image(PIL.Image.new("RGB", (60, 30), color="red")), + FileVal.as_image(PIL.Image.new("RGB", (20, 90), color="blue")), + ] + ) + self.assertEqual(exp["some"]["num"]["val"].fetch_last(), 7) + self.assertEqual(exp["some"]["str"]["val"].fetch_last(), "text") + self.assertIsInstance(exp.get_structure()["some"]["img"]["val"], FileSeries) + + def test_extend_dict(self): + exp = init(mode="debug", flush_period=0.5) + dict_value = {"key-a": ["value-a", "value-aa"], "key-b": ["value-b", "value-bb"], "key-c": ["ccc"]} + exp["some/num/val"].extend(dict_value) + self.assertEqual(exp["some"]["num"]["val"]["key-a"].fetch_last(), "value-aa") + self.assertEqual(exp["some"]["num"]["val"]["key-b"].fetch_last(), "value-bb") + self.assertEqual(exp["some"]["num"]["val"]["key-c"].fetch_last(), "ccc") + + def test_extend_complex_input(self): + exp = init(mode="debug", flush_period=0.5) + exp["train/listOfDicts"].extend([{"1a": 1, "1b": 1}, {"1a": 2, "1b": 2}]) + exp["train/listOfDictOfDicts"].extend( + [ + {"key-a": {"a1": 11, "a2": 22}}, + {"key-b": {"b1": 33, "b2": 44}}, + {"key-a": {"xx": 11, "yy": 22}}, + ] + ) + exp["train/listOfListsOfDicts"].extend( + [[{"2a": 11, "2b": 11}, {"2a": 12, "2b": 12}], [{"2a": 21, "2b": 21}, {"2a": 22, "2b": 22}]] + ) + self.assertEqual(exp["train"]["listOfDicts"].fetch_last(), "{'1a': 2, '1b': 2}") + self.assertEqual(exp["train"]["listOfDictOfDicts"].fetch_last(), "{'key-a': {'xx': 11, 'yy': 22}}") + self.assertEqual( + exp["train"]["listOfListsOfDicts"].fetch_last(), "[{'2a': 21, '2b': 21}, {'2a': 22, '2b': 22}]" + ) + def test_log_value_errors(self): exp = init(mode="debug", flush_period=0.5) img = FileVal.as_image(PIL.Image.new("RGB", (60, 30), color="red")) @@ -265,6 +373,34 @@ def setUpClass(cls) -> None: os.environ[PROJECT_ENV_NAME] = "organization/project" os.environ[API_TOKEN_ENV_NAME] = ANONYMOUS + def test_append_errors(self): + exp = init(mode="debug", flush_period=0.5) + img = FileVal.as_image(PIL.Image.new("RGB", (60, 30), color="red")) + + exp["some/num/val"].append(5, step=1) + with self.assertRaises(ValueError): + exp["some/num/val"].append("str") + with self.assertRaises(TypeError): + exp["some/num/val"].append(img) + + exp["some/str/val"].append("str", step=1) + exp["some/img/val"].append(img, step=1) + with self.assertRaises(TypeError): + exp["some/img/val"].append(5) + with self.assertRaises(FileNotFound): + exp["some/img/val"].append("path") + + self.assertEqual(exp["some"]["num"]["val"].fetch_last(), 5) + self.assertEqual(exp["some"]["str"]["val"].fetch_last(), "str") + self.assertIsInstance(exp.get_structure()["some"]["img"]["val"], FileSeries) + + def test_extend_value_errors(self): + exp = init(mode="debug", flush_period=0.5) + with self.assertRaises(NeptuneUserApiInputException): + exp["x"].extend(10, step=10) + with self.assertRaises(ValueError): + exp["x"].extend([5, "str"]) + def test_assign_set(self): exp = init(mode="debug", flush_period=0.5) exp["some/str/val"].assign(StringSetVal(["tag1", "tag2"]), wait=True) @@ -461,7 +597,7 @@ def test_attribute_error(self): with self.assertRaises(AttributeError): exp["var"].something() - def test_float_like_types(self): + def test_log_float_like_types(self): exp = init(mode="debug", flush_period=0.5) exp.define("attr1", self.FloatLike(5)) @@ -485,6 +621,19 @@ def test_float_like_types(self): with self.assertRaises(ValueError): exp["attr3"].log([4, "234a"]) + def test_append_float_like_types(self): + exp = init(mode="debug", flush_period=0.5) + exp["attr"].append(self.FloatLike(34)) + self.assertEqual(exp["attr"].fetch_last(), 34) + exp["attr"].append("345") + exp["attr"].append(self.FloatLike(34)) + exp["attr"].append(4) + exp["attr"].append(13.0) + self.assertEqual(exp["attr"].fetch_last(), 13) + with self.assertRaises(ValueError): + exp["attr"].append(4) + exp["attr"].append("234a") + def test_string_like_types(self): exp = init(mode="debug", flush_period=0.5) @@ -496,6 +645,19 @@ def test_string_like_types(self): exp["attr2"].log(["345", self.FloatLike(34), 4, 13.0]) self.assertEqual(exp["attr2"].fetch_last(), "13.0") + def test_append_string_like_types(self): + exp = init(mode="debug", flush_period=0.5) + + exp["attr1"] = "234" + exp["attr1"] = self.FloatLike(12356) + self.assertEqual(exp["attr1"].fetch(), "TestHandler.FloatLike(value=12356)") + exp["attr2"].append("xxx") + exp["attr2"].append("345") + exp["attr2"].append(self.FloatLike(34)) + exp["attr2"].append(4) + exp["attr2"].append(13.0) + self.assertEqual(exp["attr2"].fetch_last(), "13.0") + def test_object(self): class Dog: def __init__(self, name, fierceness): @@ -538,7 +700,6 @@ def test_representation(self): @dataclass class FloatLike: - value: float def __float__(self): From 74d5fb28bf6715814646b2a4ccd2951899e016ec Mon Sep 17 00:00:00 2001 From: Kluk Date: Thu, 3 Nov 2022 09:22:22 +0100 Subject: [PATCH 02/26] NPT-12400: more tests --- tests/unit/neptune/new/test_handler.py | 39 +++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/unit/neptune/new/test_handler.py b/tests/unit/neptune/new/test_handler.py index 55debbae2..e2088f6b8 100644 --- a/tests/unit/neptune/new/test_handler.py +++ b/tests/unit/neptune/new/test_handler.py @@ -597,7 +597,7 @@ def test_attribute_error(self): with self.assertRaises(AttributeError): exp["var"].something() - def test_log_float_like_types(self): + def test_float_like_types(self): exp = init(mode="debug", flush_period=0.5) exp.define("attr1", self.FloatLike(5)) @@ -634,6 +634,15 @@ def test_append_float_like_types(self): exp["attr"].append(4) exp["attr"].append("234a") + def test_extend_float_like_types(self): + exp = init(mode="debug", flush_period=0.5) + exp["attr"].extend([self.FloatLike(34)]) + self.assertEqual(exp["attr"].fetch_last(), 34) + exp["attr"].extend(["345", self.FloatLike(34), 4, 13.0]) + self.assertEqual(exp["attr"].fetch_last(), 13) + with self.assertRaises(ValueError): + exp["attr"].extend([4, "234a"]) + def test_string_like_types(self): exp = init(mode="debug", flush_period=0.5) @@ -658,6 +667,34 @@ def test_append_string_like_types(self): exp["attr2"].append(13.0) self.assertEqual(exp["attr2"].fetch_last(), "13.0") + def test_extend_string_like_types(self): + exp = init(mode="debug", flush_period=0.5) + exp["attr"].extend(["kebab"]) + exp["attr"].extend(["345", self.FloatLike(34), 4, 13.0]) + self.assertEqual(exp["attr"].fetch_last(), "13.0") + + def test_assign_dict(self): + exp = init(mode="debug", flush_period=0.5) + exp["params"] = { + "x": 5, + "metadata": {"name": "Trol", "age": 376}, + "toys": StringSeriesVal(["cudgel", "hat"]), + "nested": {"nested": {"deep_secret": FloatSeriesVal([13, 15])}}, + } + self.assertEqual(exp["params/x"].fetch(), 5) + self.assertEqual(exp["params/metadata/name"].fetch(), "Trol") + self.assertEqual(exp["params/metadata/age"].fetch(), 376) + self.assertEqual(exp["params/toys"].fetch_last(), "hat") + self.assertEqual(exp["params/nested/nested/deep_secret"].fetch_last(), 15) + + def test_convertable_to_dict(self): + exp = init(mode="debug", flush_period=0.5) + exp["params"] = argparse.Namespace(foo="bar", baz=42, nested=argparse.Namespace(nested_attr=[1, 2, 3], num=55)) + self.assertEqual(exp["params/foo"].fetch(), "bar") + self.assertEqual(exp["params/baz"].fetch(), 42) + self.assertEqual(exp["params/nested/nested_attr"].fetch(), "[1, 2, 3]") + self.assertEqual(exp["params/nested/num"].fetch(), 55) + def test_object(self): class Dog: def __init__(self, name, fierceness): From 3698fe6c4a43d58b62af83a99dfe6fd415d74cdf Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Tue, 8 Nov 2022 13:54:11 +0100 Subject: [PATCH 03/26] Fix tests --- tests/unit/neptune/new/test_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/neptune/new/test_handler.py b/tests/unit/neptune/new/test_handler.py index e2088f6b8..ba684686c 100644 --- a/tests/unit/neptune/new/test_handler.py +++ b/tests/unit/neptune/new/test_handler.py @@ -659,7 +659,7 @@ def test_append_string_like_types(self): exp["attr1"] = "234" exp["attr1"] = self.FloatLike(12356) - self.assertEqual(exp["attr1"].fetch(), "TestHandler.FloatLike(value=12356)") + self.assertEqual(exp["attr1"].fetch(), "TestOtherBehaviour.FloatLike(value=12356)") exp["attr2"].append("xxx") exp["attr2"].append("345") exp["attr2"].append(self.FloatLike(34)) From e83bfaa25d861f2d4961bbb3974b2d1cda01e9e3 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Tue, 15 Nov 2022 19:05:27 +0100 Subject: [PATCH 04/26] Adapt Series attribute to list of steps and timestamps --- .../new/attributes/series/file_series.py | 30 ++++---- .../new/attributes/series/float_series.py | 12 ++-- src/neptune/new/attributes/series/series.py | 71 +++++++++++++++---- .../new/attributes/series/string_series.py | 33 ++++----- src/neptune/new/types/series/series.py | 3 + src/neptune/new/types/series/string_series.py | 10 ++- 6 files changed, 108 insertions(+), 51 deletions(-) diff --git a/src/neptune/new/attributes/series/file_series.py b/src/neptune/new/attributes/series/file_series.py index 619860f9d..0e9e60fb1 100644 --- a/src/neptune/new/attributes/series/file_series.py +++ b/src/neptune/new/attributes/series/file_series.py @@ -35,30 +35,29 @@ ) from neptune.new.internal.types.file_types import FileType from neptune.new.internal.utils import base64_encode -from neptune.new.internal.utils.iteration import get_batches from neptune.new.internal.utils.limits import image_size_exceeds_limit_for_logging from neptune.new.types import File from neptune.new.types.series.file_series import FileSeries as FileSeriesVal Val = FileSeriesVal Data = File +LogOperation = LogImages -class FileSeries(Series[Val, Data]): - def _get_log_operations_from_value(self, value: Val, step: Optional[float], timestamp: float) -> List[Operation]: - values = [ - LogImages.ValueType( - ImageValue( - data=self._get_base64_image_content(val), - name=value.name, - description=value.description, - ), - step=step, - ts=timestamp, +class FileSeries(Series[Val, Data, LogOperation]): + MAX_BATCH_SIZE = 1 + operation_cls = LogOperation + + @classmethod + def _map_series_val(cls, value: Val) -> List[ImageValue]: + return [ + ImageValue( + data=cls._get_base64_image_content(val), + name=value.name, + description=value.description, ) for val in value.values ] - return [LogImages(self._path, chunk) for chunk in get_batches(values, batch_size=1)] def _get_clear_operation(self) -> Operation: return ClearImageLog(self._path) @@ -69,6 +68,11 @@ def _data_to_value(self, values: Iterable, **kwargs) -> Val: def _is_value_type(self, value) -> bool: return isinstance(value, FileSeriesVal) + @classmethod + def _map_log_value(cls, value): + # TODO: maybe should be moved to operations + return cls._get_base64_image_content(value) + @staticmethod def _get_base64_image_content(file: File) -> str: if file.file_type is FileType.LOCAL_FILE: diff --git a/src/neptune/new/attributes/series/float_series.py b/src/neptune/new/attributes/series/float_series.py index 09483b152..59517465e 100644 --- a/src/neptune/new/attributes/series/float_series.py +++ b/src/neptune/new/attributes/series/float_series.py @@ -15,7 +15,6 @@ # from typing import ( Iterable, - List, Optional, Union, ) @@ -30,15 +29,18 @@ Operation, ) from neptune.new.internal.utils import verify_type -from neptune.new.internal.utils.iteration import get_batches from neptune.new.internal.utils.logger import logger from neptune.new.types.series.float_series import FloatSeries as FloatSeriesVal Val = FloatSeriesVal Data = Union[float, int] +LogOperation = LogFloats -class FloatSeries(Series[Val, Data], FetchableSeries[FloatSeriesValues]): +class FloatSeries(Series[Val, Data, LogOperation], FetchableSeries[FloatSeriesValues]): + MAX_BATCH_SIZE = 100 + operation_cls = LogOperation + def configure( self, min: Optional[Union[float, int]] = None, @@ -52,10 +54,6 @@ def configure( with self._container.lock(): self._enqueue_operation(ConfigFloatSeries(self._path, min, max, unit), wait) - def _get_log_operations_from_value(self, value: Val, step: Optional[float], timestamp: float) -> List[Operation]: - values = [LogFloats.ValueType(val, step=step, ts=timestamp) for val in value.values] - return [LogFloats(self._path, chunk) for chunk in get_batches(values, batch_size=100)] - def _get_clear_operation(self) -> Operation: return ClearFloatLog(self._path) diff --git a/src/neptune/new/attributes/series/series.py b/src/neptune/new/attributes/series/series.py index dc9c53de3..87cd383af 100644 --- a/src/neptune/new/attributes/series/series.py +++ b/src/neptune/new/attributes/series/series.py @@ -15,7 +15,9 @@ # import abc import time +from itertools import cycle from typing import ( + Collection, Generic, Iterable, List, @@ -30,25 +32,45 @@ is_collection, verify_type, ) +from neptune.new.internal.utils.iteration import get_batches from neptune.new.types.series.series import Series as SeriesVal Val = TypeVar("Val", bound=SeriesVal) Data = TypeVar("Data") +LogOperation = TypeVar("LogOperation", bound=Operation) -class Series(Attribute, Generic[Val, Data]): +class Series(Attribute, Generic[Val, Data, LogOperation]): + MAX_BATCH_SIZE = None + operation_cls: type(LogOperation) = None + def clear(self, wait: bool = False) -> None: self._clear_impl(wait) - @abc.abstractmethod - def _get_log_operations_from_value(self, value: Val, step: Optional[float], timestamp: float) -> List[Operation]: - pass - - def _get_config_operation_from_value(self, value: Val) -> Optional[Operation]: + def _get_log_operations_from_value( + self, value: Val, *, steps: Union[None, Collection[float]], timestamps: Union[None, Collection[float]] + ) -> List[LogOperation]: + if steps is None: + steps = cycle([None]) + if timestamps is None: + timestamps = cycle([time.time()]) + + mapped_values = self._map_series_val(value) + values_with_step_and_ts = zip(mapped_values, steps, timestamps) + log_values = [self.operation_cls.ValueType(val, step=step, ts=ts) for val, step, ts in values_with_step_and_ts] + return [ + self.operation_cls(self._path, chunk) for chunk in get_batches(log_values, batch_size=self.MAX_BATCH_SIZE) + ] + + @classmethod + def _map_series_val(cls, value: Val) -> List[Data]: + return [value for value in value.values] + + def _get_config_operation_from_value(self, value: Val) -> Optional[LogOperation]: return None @abc.abstractmethod - def _get_clear_operation(self) -> Operation: + def _get_clear_operation(self) -> LogOperation: pass @abc.abstractmethod @@ -71,8 +93,7 @@ def assign(self, value, wait: bool = False) -> None: self._enqueue_operation(clear_op, wait=wait) else: self._enqueue_operation(clear_op, wait=False) - ts = time.time() - ops = self._get_log_operations_from_value(value, None, ts) + ops = self._get_log_operations_from_value(value, steps=None, timestamps=None) for op in ops: self._enqueue_operation(op, wait=wait) @@ -84,6 +105,7 @@ def log( wait: bool = False, **kwargs, ) -> None: + """log is a deprecated method, this code should be removed in future""" if is_collection(value): if step is not None and len(value) > 1: raise ValueError("Collection of values are not supported for explicitly defined 'step'.") @@ -96,10 +118,35 @@ def log( if timestamp is not None: verify_type("timestamp", timestamp, (float, int)) - if not timestamp: - timestamp = time.time() + steps = None if step is None else [step] + timestamps = None if timestamp is None else [timestamp] * len(value) - ops = self._get_log_operations_from_value(value, step, timestamp) + ops = self._get_log_operations_from_value(value, steps=steps, timestamps=timestamps) + + with self._container.lock(): + for op in ops: + self._enqueue_operation(op, wait) + + def extend( + self, + values: Collection[Data], + steps: Optional[Collection[float]] = None, + timestamps: Optional[Collection[float]] = None, + wait: bool = False, + **kwargs, + ) -> None: + value = self._data_to_value(values, **kwargs) + + if steps is not None: + verify_type("step", steps, (float, int)) + if len(steps) != len(values): + raise ValueError("Mismatch in len") + if timestamps is not None: + verify_type("timestamp", timestamps, (float, int)) + if len(timestamps) != len(values): + raise ValueError("Mismatch in len") + + ops = self._get_log_operations_from_value(value, steps=steps, timestamps=timestamps) with self._container.lock(): for op in ops: diff --git a/src/neptune/new/attributes/series/string_series.py b/src/neptune/new/attributes/series/string_series.py index 270a241a5..518431d4a 100644 --- a/src/neptune/new/attributes/series/string_series.py +++ b/src/neptune/new/attributes/series/string_series.py @@ -17,7 +17,6 @@ TYPE_CHECKING, Iterable, List, - Optional, ) from neptune.new.attributes.series.fetchable_series import FetchableSeries @@ -28,9 +27,9 @@ LogStrings, Operation, ) -from neptune.new.internal.utils.iteration import get_batches from neptune.new.internal.utils.logger import logger from neptune.new.internal.utils.paths import path_to_str +from neptune.new.types.series.string_series import MAX_STRING_SERIES_VALUE_LENGTH from neptune.new.types.series.string_series import StringSeries as StringSeriesVal if TYPE_CHECKING: @@ -38,18 +37,26 @@ Val = StringSeriesVal Data = str +LogOperation = LogStrings -MAX_STRING_SERIES_VALUE_LENGTH = 1000 +class StringSeries(Series[Val, Data, LogOperation], FetchableSeries[StringSeriesValues]): + MAX_BATCH_SIZE = 10 + operation_cls = LogOperation -class StringSeries(Series[Val, Data], FetchableSeries[StringSeriesValues]): def __init__(self, container: "MetadataContainer", path: List[str]): super().__init__(container, path) self._value_truncation_occurred = False - def _get_log_operations_from_value(self, value: Val, step: Optional[float], timestamp: float) -> List[Operation]: - values = [v[:MAX_STRING_SERIES_VALUE_LENGTH] for v in value.values] - if not self._value_truncation_occurred and any([len(v) > MAX_STRING_SERIES_VALUE_LENGTH for v in value.values]): + def _get_clear_operation(self) -> Operation: + return ClearStringLog(self._path) + + def _data_to_value(self, values: Iterable, **kwargs) -> Val: + if kwargs: + logger.warning("Warning: unexpected arguments (%s) in StringSeries", kwargs) + + value = StringSeriesVal(values) + if not self._value_truncation_occurred and value.truncated: # the first truncation self._value_truncation_occurred = True logger.warning( @@ -59,17 +66,7 @@ def _get_log_operations_from_value(self, value: Val, step: Optional[float], time path_to_str(self._path), MAX_STRING_SERIES_VALUE_LENGTH, ) - - values = [LogStrings.ValueType(val, step=step, ts=timestamp) for val in values] - return [LogStrings(self._path, chunk) for chunk in get_batches(values, batch_size=10)] - - def _get_clear_operation(self) -> Operation: - return ClearStringLog(self._path) - - def _data_to_value(self, values: Iterable, **kwargs) -> Val: - if kwargs: - logger.warning("Warning: unexpected arguments (%s) in StringSeries", kwargs) - return StringSeriesVal(values) + return value def _is_value_type(self, value) -> bool: return isinstance(value, StringSeriesVal) diff --git a/src/neptune/new/types/series/series.py b/src/neptune/new/types/series/series.py index 73ab12fd6..9cd1f03b1 100644 --- a/src/neptune/new/types/series/series.py +++ b/src/neptune/new/types/series/series.py @@ -36,3 +36,6 @@ def accept(self, visitor: "ValueVisitor[Ret]") -> Ret: @abc.abstractmethod def values(self): pass + + def __len__(self): + return len(self.values) diff --git a/src/neptune/new/types/series/string_series.py b/src/neptune/new/types/series/string_series.py index 75f1e2875..cb6c050df 100644 --- a/src/neptune/new/types/series/string_series.py +++ b/src/neptune/new/types/series/string_series.py @@ -27,12 +27,16 @@ Ret = TypeVar("Ret") +MAX_STRING_SERIES_VALUE_LENGTH = 1000 + class StringSeries(Series): def __init__(self, values): if not is_collection(values): raise TypeError("`values` is not a collection") - self._values = [str(value) for value in values] + values_str = [str(val) for val in values] + self._truncated = any([len(value) > MAX_STRING_SERIES_VALUE_LENGTH for value in values_str]) + self._values = [str(value)[:MAX_STRING_SERIES_VALUE_LENGTH] for value in values_str] def accept(self, visitor: "ValueVisitor[Ret]") -> Ret: return visitor.visit_string_series(self) @@ -41,5 +45,9 @@ def accept(self, visitor: "ValueVisitor[Ret]") -> Ret: def values(self): return self._values + @property + def truncated(self): + return self._truncated + def __str__(self): return "StringSeries({})".format(str(self.values)) From e3c737206d32a446d21de8f3e4f8e5d39d951dd1 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Wed, 16 Nov 2022 15:57:16 +0100 Subject: [PATCH 05/26] Delegate Container.define call to Handler --- src/neptune/new/attributes/namespace.py | 5 ++++- src/neptune/new/handler.py | 16 ++++++++++------ .../metadata_containers/metadata_container.py | 15 ++++----------- src/neptune/new/types/type_casting.py | 3 ++- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/neptune/new/attributes/namespace.py b/src/neptune/new/attributes/namespace.py index 80734d107..fb769fde2 100644 --- a/src/neptune/new/attributes/namespace.py +++ b/src/neptune/new/attributes/namespace.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # +import argparse from collections.abc import MutableMapping from typing import ( TYPE_CHECKING, @@ -89,7 +90,9 @@ def to_dict(self) -> Dict[str, Any]: return result def assign(self, value: Union[NamespaceVal, dict, Mapping], wait: bool = False): - if not isinstance(value, NamespaceVal): + if isinstance(value, argparse.Namespace): + value = NamespaceVal(vars(value)) + elif not isinstance(value, NamespaceVal): value = NamespaceVal(value) for k, v in value.value.items(): diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 9f2091884..64884b946 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -58,7 +58,9 @@ join_paths, parse_path, ) +from neptune.new.internal.value_to_attribute_visitor import ValueToAttributeVisitor from neptune.new.types.atoms.file import File as FileVal +from neptune.new.types.type_casting import cast_value from neptune.new.types.value_copy import ValueCopy if TYPE_CHECKING: @@ -161,12 +163,14 @@ def assign(self, value, wait: bool = False) -> None: """ with self._container.lock(): attr = self._container.get_attribute(self._path) - if attr: - if isinstance(value, Handler): - value = ValueCopy(value) - attr.process_assignment(value, wait) - else: - self._container.define(self._path, value, wait) + if not attr: + neptune_value = cast_value(value) + attr = ValueToAttributeVisitor(self._container, parse_path(self._path)).visit(neptune_value) + self._container.set_attribute(self._path, attr) + + if isinstance(value, Handler): + value = ValueCopy(value) + attr.process_assignment(value, wait) @check_protected_paths def upload(self, value, wait: bool = False) -> None: diff --git a/src/neptune/new/metadata_containers/metadata_container.py b/src/neptune/new/metadata_containers/metadata_container.py index 8c5f5859f..4b209b518 100644 --- a/src/neptune/new/metadata_containers/metadata_container.py +++ b/src/neptune/new/metadata_containers/metadata_container.py @@ -65,10 +65,8 @@ in_notebook, ) from neptune.new.internal.utils.uncaught_exception_handler import instance as uncaught_exception_handler -from neptune.new.internal.value_to_attribute_visitor import ValueToAttributeVisitor from neptune.new.metadata_containers.metadata_containers_table import Table from neptune.new.types.mode import Mode -from neptune.new.types.type_casting import cast_value def ensure_not_stopped(fun): @@ -237,17 +235,12 @@ def define( value: Any, wait: bool = False, ) -> Attribute: - value = cast_value(value) - parsed_path = parse_path(path) - with self._lock: - old_attr = self._structure.get(parsed_path) - if old_attr: + old_attr = self.get_attribute(path) + if old_attr is not None: raise MetadataInconsistency("Attribute or namespace {} is already defined".format(path)) - attr = ValueToAttributeVisitor(self, parsed_path).visit(value) - self._structure.set(parsed_path, attr) - attr.process_assignment(value, wait) - return attr + + self[path].assign(value, wait=wait) def get_attribute(self, path: str) -> Optional[Attribute]: with self._lock: diff --git a/src/neptune/new/types/type_casting.py b/src/neptune/new/types/type_casting.py index b5b5d643a..5c4d43e68 100644 --- a/src/neptune/new/types/type_casting.py +++ b/src/neptune/new/types/type_casting.py @@ -18,7 +18,6 @@ from typing import Any from neptune.common.deprecation import warn_once -from neptune.new.handler import Handler from neptune.new.internal.utils import ( is_bool, is_dict_like, @@ -42,6 +41,8 @@ def cast_value(value: Any) -> Value: + from neptune.new.handler import Handler + if isinstance(value, Value): return value elif isinstance(value, Handler): From 0df9c95fce5fa2fd8af74f081eb23259c565964d Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Wed, 16 Nov 2022 16:27:54 +0100 Subject: [PATCH 06/26] Implement append using extend --- src/neptune/new/attributes/namespace.py | 9 +-- src/neptune/new/attributes/series/series.py | 5 +- src/neptune/new/handler.py | 63 ++++++++------------- src/neptune/new/types/type_casting.py | 45 ++++++++++++++- 4 files changed, 75 insertions(+), 47 deletions(-) diff --git a/src/neptune/new/attributes/namespace.py b/src/neptune/new/attributes/namespace.py index fb769fde2..c0916a47e 100644 --- a/src/neptune/new/attributes/namespace.py +++ b/src/neptune/new/attributes/namespace.py @@ -18,6 +18,7 @@ from typing import ( TYPE_CHECKING, Any, + Collection, Dict, Iterable, Iterator, @@ -67,18 +68,18 @@ def __len__(self) -> int: def __iter__(self) -> Iterator[str]: yield from self._attributes.__iter__() - def log( + def extend( self, value: Union[Data, Iterable[Data]], - step: Optional[float] = None, - timestamp: Optional[float] = None, + steps: Optional[Collection[float]] = None, + timestamps: Optional[Collection[float]] = None, wait: bool = False, **kwargs, ) -> None: if not isinstance(value, NamespaceVal): value = NamespaceVal(value) for k, v in value.value.items(): - self._container[f"{self._str_path}/{k}"].log(v, step, timestamp, wait, **kwargs) + self._container[f"{self._str_path}/{k}"].extend(v, steps, timestamps, wait, **kwargs) def to_dict(self) -> Dict[str, Any]: result = {} diff --git a/src/neptune/new/attributes/series/series.py b/src/neptune/new/attributes/series/series.py index 87cd383af..96d635afe 100644 --- a/src/neptune/new/attributes/series/series.py +++ b/src/neptune/new/attributes/series/series.py @@ -30,6 +30,7 @@ from neptune.new.internal.operation import Operation from neptune.new.internal.utils import ( is_collection, + verify_collection_type, verify_type, ) from neptune.new.internal.utils.iteration import get_batches @@ -138,11 +139,11 @@ def extend( value = self._data_to_value(values, **kwargs) if steps is not None: - verify_type("step", steps, (float, int)) + verify_collection_type("steps", steps, (float, int)) if len(steps) != len(values): raise ValueError("Mismatch in len") if timestamps is not None: - verify_type("timestamp", timestamps, (float, int)) + verify_collection_type("timestamps", timestamps, (float, int)) if len(timestamps) != len(values): raise ValueError("Mismatch in len") diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 64884b946..5a402999e 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -60,7 +60,10 @@ ) from neptune.new.internal.value_to_attribute_visitor import ValueToAttributeVisitor from neptune.new.types.atoms.file import File as FileVal -from neptune.new.types.type_casting import cast_value +from neptune.new.types.type_casting import ( + cast_value, + cast_value_for_extend, +) from neptune.new.types.value_copy import ValueCopy if TYPE_CHECKING: @@ -315,37 +318,28 @@ def append( for element in value.values(): if is_collection(element): raise NeptuneUserApiInputException("Dict value cannot be a collection, use extend method") + if step is not None: + step = [step] + if timestamp is not None: + timestamp = [timestamp] + value = self._wrap_val(value) + self._do_extend(value, step, timestamp, wait, **kwargs) + + def _wrap_val(self, value): + if not isinstance(value, Namespace) and not is_dict_like(value): + return [value] + else: + return {k: self._wrap_val(v) for k, v in value.items()} + + def _do_extend(self, values, steps, timestamps, wait, **kwargs): with self._container.lock(): attr = self._container.get_attribute(self._path) if not attr: - attr = self._create_new_attribute(value) + neptune_value = cast_value_for_extend(values) + attr = ValueToAttributeVisitor(self._container, parse_path(self._path)).visit(neptune_value) self._container.set_attribute(self._path, attr) - attr.log(value, step=step, timestamp=timestamp, wait=wait, **kwargs) - - def _create_new_attribute(self, value): - if is_float(value): - return FloatSeries(self._container, parse_path(self._path)) - elif is_dict_like(value): - return Namespace(self._container, parse_path(self._path)) - elif is_string(value): - return StringSeries(self._container, parse_path(self._path)) - elif FileVal.is_convertable(value): - return FileSeries(self._container, parse_path(self._path)) - elif is_float_like(value): - return FloatSeries(self._container, parse_path(self._path)) - elif is_string_like(value): - self._warn_casting_to_string() - return StringSeries(self._container, parse_path(self._path)) - else: - raise NeptuneUserApiInputException("Value of unsupported type {}".format(type(value))) - def _warn_casting_to_string(self): - warn_once( - message="The object you're logging will be implicitly cast to a string." - " We'll end support of this behavior in `neptune-client==1.0.0`." - " To log the object as a string, use `.log(str(object))` instead.", - stack_level=3, - ) + attr.extend(values, steps=steps, timestamps=timestamps, wait=wait, **kwargs) def extend( self, @@ -360,22 +354,11 @@ def extend( """ if is_dict_like(values): self._validate_dict_for_extend(values, steps, timestamps) - for dict_key in values.keys(): - values_size = iterable_size(values[dict_key]) - new_steps = steps if steps is not None else [None] * values_size - new_timestamps = timestamps if timestamps is not None else [None] * values_size - for (value, step, timestamp) in zip(values[dict_key], new_steps, new_timestamps): - self.append({dict_key: value}, step, timestamp, wait, **kwargs) + self._do_extend(values, steps, timestamps, wait, **kwargs) elif is_collection(values): values_size = iterable_size(values) self._validate_iterable_for_extend(values_size, steps, timestamps) - new_steps = steps if steps is not None else [None] * values_size - new_timestamps = timestamps if timestamps is not None else [None] * values_size - for (value, step, timestamp) in zip(values, new_steps, new_timestamps): - if is_collection(value) or is_dict_like(value): - self.append(str(value), step, timestamp, wait, **kwargs) - else: - self.append(value, step, timestamp, wait, **kwargs) + self._do_extend(values, steps, timestamps, wait, **kwargs) else: raise NeptuneUserApiInputException( "Value cannot be a collection." diff --git a/src/neptune/new/types/type_casting.py b/src/neptune/new/types/type_casting.py index 5c4d43e68..37d98b2ef 100644 --- a/src/neptune/new/types/type_casting.py +++ b/src/neptune/new/types/type_casting.py @@ -15,7 +15,11 @@ # import argparse from datetime import datetime -from typing import Any +from typing import ( + Any, + List, + Union, +) from neptune.common.deprecation import warn_once from neptune.new.internal.utils import ( @@ -36,6 +40,12 @@ from neptune.new.types.atoms.float import Float from neptune.new.types.atoms.string import String from neptune.new.types.namespace import Namespace +from neptune.new.types.series import ( + FileSeries, + FloatSeries, + StringSeries, +) +from neptune.new.types.series.series import Series from neptune.new.types.value import Value from neptune.new.types.value_copy import ValueCopy @@ -77,3 +87,36 @@ def cast_value(value: Any) -> Value: return String(str(value)) else: raise TypeError("Value of unsupported type {}".format(type(value))) + + +def cast_value_for_extend(values: Union[Any, List[Any]]) -> Union[Series, Namespace]: + if isinstance(values, Namespace): + return values + if isinstance(values, Series): + return values + elif is_dict_like(values): + return Namespace(values) + + assert values + sample_val = values[0] + if isinstance(sample_val, File): + return FileSeries(values=values) + elif File.is_convertable_to_image(sample_val): + return FileSeries(values=values) + elif File.is_convertable_to_html(sample_val): + return FileSeries(values=values) + elif is_string(sample_val): + return StringSeries(values=values) + elif is_float_like(sample_val): + return FloatSeries(values=values) + elif is_string_like(sample_val): + warn_once( + message="The object you're logging will be implicitly cast to a string." + " We'll end support of this behavior in `neptune-client==1.0.0`." + " To log the object as a string, use `str(object)` instead.", + stack_level=3, + ) + return StringSeries(values=values) + else: + # TODO + raise TypeError("Value of unsupported type {}".format(type(sample_val))) From c2f2bb793517f1296d09ac65aa03ffa775e934d5 Mon Sep 17 00:00:00 2001 From: Sabine Date: Thu, 17 Nov 2022 16:12:17 +0200 Subject: [PATCH 07/26] Add docstring --- src/neptune/new/handler.py | 61 +++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 5a402999e..f58eac29b 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -307,8 +307,34 @@ def append( wait: bool = False, **kwargs, ) -> None: - """ - todo: NPT-12530 + """Logs a series of values, such as a metric, by appending the provided value to the end of the series. + + Available for following series field types: + + * `FloatSeries` - series of float values + * `StringSeries` - series of strings + * `FileSeries` - series of files + + When you log the first value, the type of the value determines what type of field is created. + For more, see the field types documentation: https://docs.neptune.ai/api/field_types + + Args: + value: Value to be added to the series field. + step: Optional index of the entry being appended. Must be strictly increasing. + timestamp: Optional time index of the log entry being appended, in Unix time format. + If None, the current time (obtained with `time.time()`) is used. + wait: If True, the client sends all tracked metadata to the server before executing the call. + For details, see https://docs.neptune.ai/api/universal/#wait + + Examples: + >>> import neptune.new as neptune + >>> run = neptune.init_run() + >>> for epoch in range(n_epochs): + ... ... # Your training loop + ... run["train/epoch/loss"].append(loss) # FloatSeries + ... token = str(...) + ... run["train/tokens"].append(token) # StringSeries + ... run["train/distribution"].append(plt_histogram, step=epoch) # FileSeries """ verify_type("step", step, (int, float, type(None))) verify_type("timestamp", timestamp, (int, float, type(None))) @@ -349,8 +375,35 @@ def extend( wait: bool = False, **kwargs, ) -> None: - """ - todo: NPT-12530 + """Logs a series of values by appending the provided collection of values to the end of the series. + + Available for following series field types: + + * `FloatSeries` - series of float values + * `StringSeries` - series of strings + * `FileSeries` - series of files + + When you log the first value, the type of the value determines what type of field is created. + For more, see the field types documentation: https://docs.neptune.ai/api/field_types + + Args: + values: Values to be added to the series field, as a dictionary or collection. + steps: Optional collection of indeces for the entries being appended. Must be strictly increasing. + timestamps: Optional collection of time indeces for the entries being appended, in Unix time format. + If None, the current time (obtained with `time.time()`) is used. + wait: If True, the client sends all tracked metadata to the server before executing the call. + For details, see https://docs.neptune.ai/api/universal/#wait + + Example: + The following example reads a CSV file into a pandas DataFrame and extracts the values + to create a Neptune series. + >>> import neptune.new as neptune + >>> run = neptune.init_run() + >>> for epoch in range(n_epochs): + ... df = pandas.read_csv("time_series.csv") + ... ys = df["value"] + ... ts = df["timestamp"] + ... run["data/example_series"].extend(ys, timestamps=ts) """ if is_dict_like(values): self._validate_dict_for_extend(values, steps, timestamps) From 270469f46d1dff0940f21d1134122b1a4d7757a6 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Thu, 17 Nov 2022 17:11:45 +0100 Subject: [PATCH 08/26] Keep maximum one nested level for `append` --- src/neptune/new/attributes/series/series.py | 6 ++---- src/neptune/new/handler.py | 17 ++++++++++------- src/neptune/new/types/type_casting.py | 11 +++++------ 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/neptune/new/attributes/series/series.py b/src/neptune/new/attributes/series/series.py index 96d635afe..0c8bfe737 100644 --- a/src/neptune/new/attributes/series/series.py +++ b/src/neptune/new/attributes/series/series.py @@ -140,12 +140,10 @@ def extend( if steps is not None: verify_collection_type("steps", steps, (float, int)) - if len(steps) != len(values): - raise ValueError("Mismatch in len") + assert len(steps) == len(values) if timestamps is not None: verify_collection_type("timestamps", timestamps, (float, int)) - if len(timestamps) != len(values): - raise ValueError("Mismatch in len") + assert len(timestamps) == len(values) ops = self._get_log_operations_from_value(value, steps=steps, timestamps=timestamps) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index f58eac29b..54a0b8d94 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -348,14 +348,17 @@ def append( step = [step] if timestamp is not None: timestamp = [timestamp] - value = self._wrap_val(value) + value = self._transform_to_extend_format(value) self._do_extend(value, step, timestamp, wait, **kwargs) - def _wrap_val(self, value): - if not isinstance(value, Namespace) and not is_dict_like(value): - return [value] + def _transform_to_extend_format(self, value, *, depth=1): + """Preserve nested structure created by `Namespaces` and `dict_like` objects, + but replace all other values with single-element lists, + so work can be delegated to `_do_extend` method.""" + if depth == 1 and (isinstance(value, Namespace) or is_dict_like(value)): + return {k: self._transform_to_extend_format(v, depth=depth + 1) for k, v in value.items()} else: - return {k: self._wrap_val(v) for k, v in value.items()} + return [value] def _do_extend(self, values, steps, timestamps, wait, **kwargs): with self._container.lock(): @@ -405,8 +408,8 @@ def extend( ... ts = df["timestamp"] ... run["data/example_series"].extend(ys, timestamps=ts) """ - if is_dict_like(values): - self._validate_dict_for_extend(values, steps, timestamps) + if isinstance(values, Namespace) or is_dict_like(values): + # self._validate_dict_for_extend(values, steps, timestamps) self._do_extend(values, steps, timestamps, wait, **kwargs) elif is_collection(values): values_size = iterable_size(values) diff --git a/src/neptune/new/types/type_casting.py b/src/neptune/new/types/type_casting.py index 37d98b2ef..49e6cb0e8 100644 --- a/src/neptune/new/types/type_casting.py +++ b/src/neptune/new/types/type_casting.py @@ -89,15 +89,15 @@ def cast_value(value: Any) -> Value: raise TypeError("Value of unsupported type {}".format(type(value))) -def cast_value_for_extend(values: Union[Any, List[Any]]) -> Union[Series, Namespace]: +def cast_value_for_extend(values: Union[Namespace, Series, List[Any]]) -> Union[Series, Namespace]: if isinstance(values, Namespace): return values - if isinstance(values, Series): - return values elif is_dict_like(values): return Namespace(values) + elif isinstance(values, Series): + return values - assert values + assert values # assert some value was passed, otherwise we can't determine the type sample_val = values[0] if isinstance(sample_val, File): return FileSeries(values=values) @@ -118,5 +118,4 @@ def cast_value_for_extend(values: Union[Any, List[Any]]) -> Union[Series, Namesp ) return StringSeries(values=values) else: - # TODO - raise TypeError("Value of unsupported type {}".format(type(sample_val))) + raise TypeError("Value of unsupported type List[{}]".format(type(sample_val))) From c7967462e85e72cf76e4921918720881d7503166 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Thu, 17 Nov 2022 18:23:23 +0100 Subject: [PATCH 09/26] Small fixes --- src/neptune/new/attributes/namespace.py | 4 +-- src/neptune/new/attributes/series/series.py | 28 +++++++++---------- src/neptune/new/handler.py | 20 ++++++------- src/neptune/new/internal/operation.py | 12 +++++--- src/neptune/new/internal/utils/__init__.py | 4 --- src/neptune/new/types/series/string_series.py | 3 +- src/neptune/new/types/type_casting.py | 9 +++--- .../neptune/new/internal/test_operations.py | 15 ++++++++-- tests/unit/neptune/new/test_imports.py | 1 - 9 files changed, 52 insertions(+), 44 deletions(-) diff --git a/src/neptune/new/attributes/namespace.py b/src/neptune/new/attributes/namespace.py index c0916a47e..52affa798 100644 --- a/src/neptune/new/attributes/namespace.py +++ b/src/neptune/new/attributes/namespace.py @@ -29,7 +29,7 @@ ) from neptune.new.attributes.attribute import Attribute -from neptune.new.attributes.series.series import Data +from neptune.new.attributes.series.series import DataTV from neptune.new.internal.container_structure import ContainerStructure from neptune.new.internal.utils.generic_attribute_mapper import ( NoValue, @@ -70,7 +70,7 @@ def __iter__(self) -> Iterator[str]: def extend( self, - value: Union[Data, Iterable[Data]], + value: Union[DataTV, Iterable[DataTV]], steps: Optional[Collection[float]] = None, timestamps: Optional[Collection[float]] = None, wait: bool = False, diff --git a/src/neptune/new/attributes/series/series.py b/src/neptune/new/attributes/series/series.py index 0c8bfe737..dad62ba3b 100644 --- a/src/neptune/new/attributes/series/series.py +++ b/src/neptune/new/attributes/series/series.py @@ -27,7 +27,7 @@ ) from neptune.new.attributes.attribute import Attribute -from neptune.new.internal.operation import Operation +from neptune.new.internal.operation import LogOperation from neptune.new.internal.utils import ( is_collection, verify_collection_type, @@ -36,21 +36,21 @@ from neptune.new.internal.utils.iteration import get_batches from neptune.new.types.series.series import Series as SeriesVal -Val = TypeVar("Val", bound=SeriesVal) -Data = TypeVar("Data") -LogOperation = TypeVar("LogOperation", bound=Operation) +ValTV = TypeVar("ValTV", bound=SeriesVal) +DataTV = TypeVar("DataTV") +LogOperationTV = TypeVar("LogOperationTV", bound=LogOperation) -class Series(Attribute, Generic[Val, Data, LogOperation]): +class Series(Attribute, Generic[ValTV, DataTV, LogOperationTV]): MAX_BATCH_SIZE = None - operation_cls: type(LogOperation) = None + operation_cls: type(LogOperationTV) = None def clear(self, wait: bool = False) -> None: self._clear_impl(wait) def _get_log_operations_from_value( - self, value: Val, *, steps: Union[None, Collection[float]], timestamps: Union[None, Collection[float]] - ) -> List[LogOperation]: + self, value: ValTV, *, steps: Union[None, Collection[float]], timestamps: Union[None, Collection[float]] + ) -> List[LogOperationTV]: if steps is None: steps = cycle([None]) if timestamps is None: @@ -64,18 +64,18 @@ def _get_log_operations_from_value( ] @classmethod - def _map_series_val(cls, value: Val) -> List[Data]: + def _map_series_val(cls, value: ValTV) -> List[DataTV]: return [value for value in value.values] - def _get_config_operation_from_value(self, value: Val) -> Optional[LogOperation]: + def _get_config_operation_from_value(self, value: ValTV) -> Optional[LogOperationTV]: return None @abc.abstractmethod - def _get_clear_operation(self) -> LogOperation: + def _get_clear_operation(self) -> LogOperationTV: pass @abc.abstractmethod - def _data_to_value(self, values: Iterable, **kwargs) -> Val: + def _data_to_value(self, values: Iterable, **kwargs) -> ValTV: pass @abc.abstractmethod @@ -100,7 +100,7 @@ def assign(self, value, wait: bool = False) -> None: def log( self, - value: Union[Data, Iterable[Data]], + value: Union[DataTV, Iterable[DataTV]], step: Optional[float] = None, timestamp: Optional[float] = None, wait: bool = False, @@ -130,7 +130,7 @@ def log( def extend( self, - values: Collection[Data], + values: Collection[DataTV], steps: Optional[Collection[float]] = None, timestamps: Optional[Collection[float]] = None, wait: bool = False, diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 54a0b8d94..f025a59b3 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -17,6 +17,7 @@ from typing import ( TYPE_CHECKING, Any, + Collection, Dict, Iterable, List, @@ -50,7 +51,6 @@ is_float_like, is_string, is_string_like, - iterable_size, verify_collection_type, verify_type, ) @@ -352,7 +352,7 @@ def append( self._do_extend(value, step, timestamp, wait, **kwargs) def _transform_to_extend_format(self, value, *, depth=1): - """Preserve nested structure created by `Namespaces` and `dict_like` objects, + """Preserve nested structure (but only one level) created by `Namespaces` and `dict_like` objects, but replace all other values with single-element lists, so work can be delegated to `_do_extend` method.""" if depth == 1 and (isinstance(value, Namespace) or is_dict_like(value)): @@ -412,7 +412,7 @@ def extend( # self._validate_dict_for_extend(values, steps, timestamps) self._do_extend(values, steps, timestamps, wait, **kwargs) elif is_collection(values): - values_size = iterable_size(values) + values_size = len(values) self._validate_iterable_for_extend(values_size, steps, timestamps) self._do_extend(values, steps, timestamps, wait, **kwargs) else: @@ -425,23 +425,23 @@ def extend( def _validate_iterable_for_extend( self, values_size: int, steps: Optional[Iterable[float]] = None, timestamps: Optional[Iterable[float]] = None ) -> None: - if steps is not None and iterable_size(steps) != values_size: + if steps is not None and len(steps) != values_size: raise NeptuneUserApiInputException("Number of steps must be equal to number of values") - if timestamps is not None and iterable_size(timestamps) != values_size: + if timestamps is not None and len(timestamps) != values_size: raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values") def _validate_dict_for_extend( self, values: Union[Dict[str, Iterable[Any]], Iterable[Any]], - steps: Optional[Iterable[float]] = None, - timestamps: Optional[Iterable[float]] = None, + steps: Optional[Collection[float]] = None, + timestamps: Optional[Collection[float]] = None, ) -> None: - timestamps_size = None if timestamps is None else iterable_size(timestamps) - steps_size = None if steps is None else iterable_size(steps) + timestamps_size = None if timestamps is None else len(timestamps) + steps_size = None if steps is None else len(steps) for dict_key in values: if not is_collection(values[dict_key]): raise NeptuneUserApiInputException("If value is a dict, all values in dict should be a collection") - dict_iterables_size = iterable_size(values[dict_key]) + dict_iterables_size = len(values[dict_key]) if timestamps_size is not None and timestamps_size != dict_iterables_size: raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values in dict") if steps_size is not None and steps_size != dict_iterables_size: diff --git a/src/neptune/new/internal/operation.py b/src/neptune/new/internal/operation.py index 963e817db..8a507a4db 100644 --- a/src/neptune/new/internal/operation.py +++ b/src/neptune/new/internal/operation.py @@ -49,7 +49,7 @@ def all_subclasses(cls): @dataclass -class Operation: +class Operation(abc.ABC): path: List[str] @@ -276,6 +276,10 @@ def from_dict(data: dict) -> "UploadFileSet": return UploadFileSet(data["path"], data["file_globs"], data["reset"] != str(False)) +class LogOperation(Operation, abc.ABC): + pass + + @dataclass class LogSeriesValue(Generic[T]): @@ -292,7 +296,7 @@ def from_dict(data: dict, value_deserializer=lambda x: x) -> "LogSeriesValue[T]" @dataclass -class LogFloats(Operation): +class LogFloats(LogOperation): ValueType = LogSeriesValue[float] @@ -315,7 +319,7 @@ def from_dict(data: dict) -> "LogFloats": @dataclass -class LogStrings(Operation): +class LogStrings(LogOperation): ValueType = LogSeriesValue[str] @@ -360,7 +364,7 @@ def deserializer(obj) -> "ImageValue": @dataclass -class LogImages(Operation): +class LogImages(LogOperation): ValueType = LogSeriesValue[ImageValue] diff --git a/src/neptune/new/internal/utils/__init__.py b/src/neptune/new/internal/utils/__init__.py index 2f543e08e..a5959d320 100644 --- a/src/neptune/new/internal/utils/__init__.py +++ b/src/neptune/new/internal/utils/__init__.py @@ -86,10 +86,6 @@ def is_dict_like(var): return isinstance(var, (dict, Mapping)) -def iterable_size(var): - return len(var) - - def is_string_like(var): try: _ = str(var) diff --git a/src/neptune/new/types/series/string_series.py b/src/neptune/new/types/series/string_series.py index cb6c050df..d7a9b457d 100644 --- a/src/neptune/new/types/series/string_series.py +++ b/src/neptune/new/types/series/string_series.py @@ -36,7 +36,7 @@ def __init__(self, values): raise TypeError("`values` is not a collection") values_str = [str(val) for val in values] self._truncated = any([len(value) > MAX_STRING_SERIES_VALUE_LENGTH for value in values_str]) - self._values = [str(value)[:MAX_STRING_SERIES_VALUE_LENGTH] for value in values_str] + self._values = [value[:MAX_STRING_SERIES_VALUE_LENGTH] for value in values_str] def accept(self, visitor: "ValueVisitor[Ret]") -> Ret: return visitor.visit_string_series(self) @@ -47,6 +47,7 @@ def values(self): @property def truncated(self): + """True if any value had to be truncated to `MAX_STRING_SERIES_VALUE_LENGTH`""" return self._truncated def __str__(self): diff --git a/src/neptune/new/types/type_casting.py b/src/neptune/new/types/type_casting.py index 49e6cb0e8..69f3aad54 100644 --- a/src/neptune/new/types/type_casting.py +++ b/src/neptune/new/types/type_casting.py @@ -17,7 +17,7 @@ from datetime import datetime from typing import ( Any, - List, + Collection, Union, ) @@ -89,7 +89,7 @@ def cast_value(value: Any) -> Value: raise TypeError("Value of unsupported type {}".format(type(value))) -def cast_value_for_extend(values: Union[Namespace, Series, List[Any]]) -> Union[Series, Namespace]: +def cast_value_for_extend(values: Union[Namespace, Series, Collection[Any]]) -> Union[Series, Namespace]: if isinstance(values, Namespace): return values elif is_dict_like(values): @@ -97,8 +97,7 @@ def cast_value_for_extend(values: Union[Namespace, Series, List[Any]]) -> Union[ elif isinstance(values, Series): return values - assert values # assert some value was passed, otherwise we can't determine the type - sample_val = values[0] + sample_val = next(iter(values)) if isinstance(sample_val, File): return FileSeries(values=values) elif File.is_convertable_to_image(sample_val): @@ -114,7 +113,7 @@ def cast_value_for_extend(values: Union[Namespace, Series, List[Any]]) -> Union[ message="The object you're logging will be implicitly cast to a string." " We'll end support of this behavior in `neptune-client==1.0.0`." " To log the object as a string, use `str(object)` instead.", - stack_level=3, + stack_level=3, # TODO: check ) return StringSeries(values=values) else: diff --git a/tests/unit/neptune/new/internal/test_operations.py b/tests/unit/neptune/new/internal/test_operations.py index 66077e9a5..b59bd2e5d 100644 --- a/tests/unit/neptune/new/internal/test_operations.py +++ b/tests/unit/neptune/new/internal/test_operations.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # +import inspect import json import unittest import uuid @@ -53,12 +54,20 @@ class TestOperations(unittest.TestCase): def test_serialization_to_dict(self): - classes = {cls.__name__ for cls in all_subclasses(Operation)} + classes = {cls for cls in all_subclasses(Operation)} + # drop abstract classes + for cls in classes.copy(): + if inspect.isabstract(cls): + classes.remove(cls) + + # test every Operation subclass and drop from `classes` for obj in self._list_objects(): - if obj.__class__.__name__ in classes: - classes.remove(obj.__class__.__name__) + if obj.__class__ in classes: + classes.remove(obj.__class__) deserialized_obj = Operation.from_dict(json.loads(json.dumps(obj.to_dict()))) self.assertEqual(obj.__dict__, deserialized_obj.__dict__) + + # expect no Operation subclass left self.assertEqual(classes, set()) @staticmethod diff --git a/tests/unit/neptune/new/test_imports.py b/tests/unit/neptune/new/test_imports.py index cbd8836cc..3a1aacf35 100644 --- a/tests/unit/neptune/new/test_imports.py +++ b/tests/unit/neptune/new/test_imports.py @@ -161,7 +161,6 @@ Attribute, Generic, Iterable, - Operation, Series, SeriesVal, TypeVar, From 272cdd3134a464175b6c1cd7881d004e67e9b70f Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Thu, 17 Nov 2022 19:44:25 +0100 Subject: [PATCH 10/26] Create ExtendUtils --- src/neptune/new/handler.py | 79 ++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index f025a59b3..88c8dc6e7 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -348,18 +348,9 @@ def append( step = [step] if timestamp is not None: timestamp = [timestamp] - value = self._transform_to_extend_format(value) + value = ExtendUtils.transform_to_extend_format(value) self._do_extend(value, step, timestamp, wait, **kwargs) - def _transform_to_extend_format(self, value, *, depth=1): - """Preserve nested structure (but only one level) created by `Namespaces` and `dict_like` objects, - but replace all other values with single-element lists, - so work can be delegated to `_do_extend` method.""" - if depth == 1 and (isinstance(value, Namespace) or is_dict_like(value)): - return {k: self._transform_to_extend_format(v, depth=depth + 1) for k, v in value.items()} - else: - return [value] - def _do_extend(self, values, steps, timestamps, wait, **kwargs): with self._container.lock(): attr = self._container.get_attribute(self._path) @@ -409,11 +400,11 @@ def extend( ... run["data/example_series"].extend(ys, timestamps=ts) """ if isinstance(values, Namespace) or is_dict_like(values): - # self._validate_dict_for_extend(values, steps, timestamps) + ExtendUtils.validate_dict_for_extend(values, steps, timestamps) self._do_extend(values, steps, timestamps, wait, **kwargs) elif is_collection(values): values_size = len(values) - self._validate_iterable_for_extend(values_size, steps, timestamps) + ExtendUtils.validate_iterable_for_extend(values_size, steps, timestamps) self._do_extend(values, steps, timestamps, wait, **kwargs) else: raise NeptuneUserApiInputException( @@ -422,31 +413,6 @@ def extend( " run['series'].extend(collection)" ) - def _validate_iterable_for_extend( - self, values_size: int, steps: Optional[Iterable[float]] = None, timestamps: Optional[Iterable[float]] = None - ) -> None: - if steps is not None and len(steps) != values_size: - raise NeptuneUserApiInputException("Number of steps must be equal to number of values") - if timestamps is not None and len(timestamps) != values_size: - raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values") - - def _validate_dict_for_extend( - self, - values: Union[Dict[str, Iterable[Any]], Iterable[Any]], - steps: Optional[Collection[float]] = None, - timestamps: Optional[Collection[float]] = None, - ) -> None: - timestamps_size = None if timestamps is None else len(timestamps) - steps_size = None if steps is None else len(steps) - for dict_key in values: - if not is_collection(values[dict_key]): - raise NeptuneUserApiInputException("If value is a dict, all values in dict should be a collection") - dict_iterables_size = len(values[dict_key]) - if timestamps_size is not None and timestamps_size != dict_iterables_size: - raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values in dict") - if steps_size is not None and steps_size != dict_iterables_size: - raise NeptuneUserApiInputException("Number of steps must be equal to number of values in dict") - @check_protected_paths def add(self, values: Union[str, Iterable[str]], wait: bool = False) -> None: """Adds the provided tag or tags to the run's tags. @@ -685,3 +651,42 @@ def track_files(self, path: str, destination: str = None, wait: bool = False) -> def __delitem__(self, path) -> None: self.pop(path) + + +class ExtendUtils: + @staticmethod + def transform_to_extend_format(value, *, depth=1): + """Preserve nested structure (but only one level) created by `Namespaces` and `dict_like` objects, + but replace all other values with single-element lists, + so work can be delegated to `_do_extend` method.""" + if depth == 1 and (isinstance(value, Namespace) or is_dict_like(value)): + return {k: ExtendUtils.transform_to_extend_format(v, depth=depth + 1) for k, v in value.items()} + else: + return [value] + + @staticmethod + def validate_iterable_for_extend( + values_size: int, steps: Optional[Collection[float]] = None, timestamps: Optional[Collection[float]] = None + ) -> None: + if steps is not None and len(steps) != values_size: + raise NeptuneUserApiInputException("Number of steps must be equal to number of values") + if timestamps is not None and len(timestamps) != values_size: + raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values") + + @staticmethod + def validate_dict_for_extend( + values: Union[Dict[str, Collection[Any]], Collection[Any]], + steps: Optional[Collection[float]] = None, + timestamps: Optional[Collection[float]] = None, + ) -> None: + """Here's business requirement that we'll support only one level of nesting when calling `extend` function""" + timestamps_size = None if timestamps is None else len(timestamps) + steps_size = None if steps is None else len(steps) + for dict_key in values: + if not is_collection(values[dict_key]): + raise NeptuneUserApiInputException("If value is a dict, all values in dict should be a collection") + dict_iterables_size = len(values[dict_key]) + if timestamps_size is not None and timestamps_size != dict_iterables_size: + raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values in dict") + if steps_size is not None and steps_size != dict_iterables_size: + raise NeptuneUserApiInputException("Number of steps must be equal to number of values in dict") From d5eae36abd05d8b2df4482c33de3ada5b78cfd5f Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Fri, 18 Nov 2022 15:31:07 +0100 Subject: [PATCH 11/26] Adjust stack_level --- src/neptune/new/handler.py | 1 + src/neptune/new/types/type_casting.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 88c8dc6e7..8f850aa21 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -170,6 +170,7 @@ def assign(self, value, wait: bool = False) -> None: neptune_value = cast_value(value) attr = ValueToAttributeVisitor(self._container, parse_path(self._path)).visit(neptune_value) self._container.set_attribute(self._path, attr) + value = neptune_value # TODO: we should clean some rules regarding type casting in assignment if isinstance(value, Handler): value = ValueCopy(value) diff --git a/src/neptune/new/types/type_casting.py b/src/neptune/new/types/type_casting.py index 69f3aad54..a39a1193d 100644 --- a/src/neptune/new/types/type_casting.py +++ b/src/neptune/new/types/type_casting.py @@ -82,7 +82,7 @@ def cast_value(value: Any) -> Value: message="The object you're logging will be implicitly cast to a string." " We'll end support of this behavior in `neptune-client==1.0.0`." " To log the object as a string, use `str(object)` instead.", - stack_level=3, + stack_level=6, ) return String(str(value)) else: @@ -113,7 +113,7 @@ def cast_value_for_extend(values: Union[Namespace, Series, Collection[Any]]) -> message="The object you're logging will be implicitly cast to a string." " We'll end support of this behavior in `neptune-client==1.0.0`." " To log the object as a string, use `str(object)` instead.", - stack_level=3, # TODO: check + stack_level=4, ) return StringSeries(values=values) else: From 608cb74cb7ac17d8684943dccb43afca2cb79e33 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Fri, 18 Nov 2022 21:46:51 +0100 Subject: [PATCH 12/26] Allow arbitrary deep nested level for series --- src/neptune/new/handler.py | 77 ++++++++++++-------------- tests/unit/neptune/new/test_handler.py | 61 ++++++++++++++++++-- 2 files changed, 90 insertions(+), 48 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 8f850aa21..53c097c97 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -17,9 +17,9 @@ from typing import ( TYPE_CHECKING, Any, - Collection, Dict, Iterable, + Iterator, List, Optional, Union, @@ -353,6 +353,8 @@ def append( self._do_extend(value, step, timestamp, wait, **kwargs) def _do_extend(self, values, steps, timestamps, wait, **kwargs): + ExtendUtils.validate_values_for_extend(values, steps, timestamps) + with self._container.lock(): attr = self._container.get_attribute(self._path) if not attr: @@ -400,19 +402,7 @@ def extend( ... ts = df["timestamp"] ... run["data/example_series"].extend(ys, timestamps=ts) """ - if isinstance(values, Namespace) or is_dict_like(values): - ExtendUtils.validate_dict_for_extend(values, steps, timestamps) - self._do_extend(values, steps, timestamps, wait, **kwargs) - elif is_collection(values): - values_size = len(values) - ExtendUtils.validate_iterable_for_extend(values_size, steps, timestamps) - self._do_extend(values, steps, timestamps, wait, **kwargs) - else: - raise NeptuneUserApiInputException( - "Value cannot be a collection." - " To append multiple values at once, use extend() instead:" - " run['series'].extend(collection)" - ) + self._do_extend(values, steps, timestamps, wait, **kwargs) @check_protected_paths def add(self, values: Union[str, Iterable[str]], wait: bool = False) -> None: @@ -656,38 +646,41 @@ def __delitem__(self, path) -> None: class ExtendUtils: @staticmethod - def transform_to_extend_format(value, *, depth=1): - """Preserve nested structure (but only one level) created by `Namespaces` and `dict_like` objects, + def transform_to_extend_format(value): + """Preserve nested structure created by `Namespaces` and `dict_like` objects, but replace all other values with single-element lists, so work can be delegated to `_do_extend` method.""" - if depth == 1 and (isinstance(value, Namespace) or is_dict_like(value)): - return {k: ExtendUtils.transform_to_extend_format(v, depth=depth + 1) for k, v in value.items()} - else: + if not isinstance(value, Namespace) and not is_dict_like(value): return [value] + else: + return {k: ExtendUtils.transform_to_extend_format(v) for k, v in value.items()} @staticmethod - def validate_iterable_for_extend( - values_size: int, steps: Optional[Collection[float]] = None, timestamps: Optional[Collection[float]] = None - ) -> None: - if steps is not None and len(steps) != values_size: - raise NeptuneUserApiInputException("Number of steps must be equal to number of values") - if timestamps is not None and len(timestamps) != values_size: - raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values") + def validate_values_for_extend(values, steps, timestamps): + collections_lengths = list(ExtendUtils.generate_leaf_collection_lengths(values)) + + if len(collections_lengths) > 1: + if steps is not None: + raise NeptuneUserApiInputException("Number of steps must be equal to number of values") + if timestamps is not None: + raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values") + else: + common_collections_length = next(iter(collections_lengths)) + if steps is not None and common_collections_length != len(steps): + raise NeptuneUserApiInputException("Number of steps must be equal to number of values") + if timestamps is not None and common_collections_length != len(timestamps): + raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values") @staticmethod - def validate_dict_for_extend( - values: Union[Dict[str, Collection[Any]], Collection[Any]], - steps: Optional[Collection[float]] = None, - timestamps: Optional[Collection[float]] = None, - ) -> None: - """Here's business requirement that we'll support only one level of nesting when calling `extend` function""" - timestamps_size = None if timestamps is None else len(timestamps) - steps_size = None if steps is None else len(steps) - for dict_key in values: - if not is_collection(values[dict_key]): - raise NeptuneUserApiInputException("If value is a dict, all values in dict should be a collection") - dict_iterables_size = len(values[dict_key]) - if timestamps_size is not None and timestamps_size != dict_iterables_size: - raise NeptuneUserApiInputException("Number of timestamps must be equal to number of values in dict") - if steps_size is not None and steps_size != dict_iterables_size: - raise NeptuneUserApiInputException("Number of steps must be equal to number of values in dict") + def generate_leaf_collection_lengths(values) -> Iterator[int]: + if isinstance(values, Namespace) or is_dict_like(values): + for val in values.values(): + yield from ExtendUtils.generate_leaf_collection_lengths(val) + elif is_collection(values): + yield len(values) + else: + raise NeptuneUserApiInputException( + "Value must be a collection or dict of collections." + " To append multiple values at once, use extend() instead:" + " run['series'].extend(collection)" + ) diff --git a/tests/unit/neptune/new/test_handler.py b/tests/unit/neptune/new/test_handler.py index ba684686c..cd6c914fe 100644 --- a/tests/unit/neptune/new/test_handler.py +++ b/tests/unit/neptune/new/test_handler.py @@ -15,6 +15,7 @@ # import argparse import os +import time import unittest from dataclasses import dataclass from datetime import ( @@ -258,8 +259,10 @@ def test_append_complex_input(self): "key-b": {"ba": 33, "bb": 44}, } ) - self.assertEqual(exp["train"]["dictOfDicts"]["key-a"].fetch_last(), "{'aa': 11, 'ab': 22}") - self.assertEqual(exp["train"]["dictOfDicts"]["key-b"].fetch_last(), "{'ba': 33, 'bb': 44}") + self.assertEqual(exp["train"]["dictOfDicts"]["key-a"]["aa"].fetch_last(), 11) + self.assertEqual(exp["train"]["dictOfDicts"]["key-a"]["ab"].fetch_last(), 22) + self.assertEqual(exp["train"]["dictOfDicts"]["key-b"]["ba"].fetch_last(), 33) + self.assertEqual(exp["train"]["dictOfDicts"]["key-b"]["bb"].fetch_last(), 44) def test_log_many_values(self): exp = init(mode="debug", flush_period=0.5) @@ -315,18 +318,22 @@ def test_extend_dict(self): self.assertEqual(exp["some"]["num"]["val"]["key-b"].fetch_last(), "value-bb") self.assertEqual(exp["some"]["num"]["val"]["key-c"].fetch_last(), "ccc") - def test_extend_complex_input(self): + def test_extend_dict_in_list(self): + """We expect that everything which is inside Collection is mapped to atoms""" exp = init(mode="debug", flush_period=0.5) - exp["train/listOfDicts"].extend([{"1a": 1, "1b": 1}, {"1a": 2, "1b": 2}]) + exp["train/listOfDicts"].extend([{"1a": 1, "1b": 1}, {"1a": 2, "1b": 2}]) # inside list -> mapped to str exp["train/listOfDictOfDicts"].extend( [ - {"key-a": {"a1": 11, "a2": 22}}, + {"key-a": {"a1": 11, "a2": 22}}, # inside list -> mapped to str {"key-b": {"b1": 33, "b2": 44}}, {"key-a": {"xx": 11, "yy": 22}}, ] ) exp["train/listOfListsOfDicts"].extend( - [[{"2a": 11, "2b": 11}, {"2a": 12, "2b": 12}], [{"2a": 21, "2b": 21}, {"2a": 22, "2b": 22}]] + [ + [{"2a": 11, "2b": 11}, {"2a": 12, "2b": 12}], # inside list -> mapped to str + [{"2a": 21, "2b": 21}, {"2a": 22, "2b": 22}], + ] ) self.assertEqual(exp["train"]["listOfDicts"].fetch_last(), "{'1a': 2, '1b': 2}") self.assertEqual(exp["train"]["listOfDictOfDicts"].fetch_last(), "{'key-a': {'xx': 11, 'yy': 22}}") @@ -334,6 +341,48 @@ def test_extend_complex_input(self): exp["train"]["listOfListsOfDicts"].fetch_last(), "[{'2a': 21, '2b': 21}, {'2a': 22, '2b': 22}]" ) + def test_extend_nested(self): + """We expect that we are able to log arbitrary tre structure""" + exp = init(mode="debug", flush_period=0.5) + + exp["train/simple_dict"].extend({"list1": [1, 2, 3], "list2": [10, 20, 30]}) + exp["train/simple_dict"].extend( + { + "list1": [4, 5, 6], + } + ) + self.assertEqual(exp["train"]["simple_dict"]["list1"].fetch_last(), 6) + self.assertEqual(list(exp["train"]["simple_dict"]["list1"].fetch_values().value), [1, 2, 3, 4, 5, 6]) + self.assertEqual(exp["train"]["simple_dict"]["list2"].fetch_last(), 30) + self.assertEqual(list(exp["train"]["simple_dict"]["list2"].fetch_values().value), [10, 20, 30]) + + exp["train/different-depths"].extend( + {"lvl1": {"lvl1.1": [1, 2, 3], "lvl1.2": {"lvl1.2.1": [1]}}, "lvl2": [10, 20]} + ) + exp["train/different-depths/lvl1"].extend({"lvl1.2": {"lvl1.2.1": [2, 3]}}) + self.assertEqual(exp["train"]["different-depths"]["lvl1"]["lvl1.1"].fetch_last(), 3) + self.assertEqual(list(exp["train"]["different-depths"]["lvl1"]["lvl1.1"].fetch_values().value), [1, 2, 3]) + self.assertEqual(exp["train"]["different-depths"]["lvl1"]["lvl1.2"]["lvl1.2.1"].fetch_last(), 3) + self.assertEqual( + list(exp["train"]["different-depths"]["lvl1"]["lvl1.2"]["lvl1.2.1"].fetch_values().value), [1, 2, 3] + ) + self.assertEqual(exp["train"]["different-depths"]["lvl2"].fetch_last(), 20) + self.assertEqual(list(exp["train"]["different-depths"]["lvl2"].fetch_values().value), [10, 20]) + + def test_extend_nested_with_wrong_parameters(self): + """We expect that we are able to log arbitrary tre structure""" + exp = init(mode="debug", flush_period=0.5) + + with self.assertRaises(NeptuneUserApiInputException): + # wrong number of steps + exp["train/simple_dict"].extend(values={"list1": [1, 2, 3], "list2": [10, 20, 30]}, steps=[0, 1]) + + with self.assertRaises(NeptuneUserApiInputException): + # wrong number of timestamps + exp["train/simple_dict"].extend( + values={"list1": [1, 2, 3], "list2": [10, 20, 30]}, timestamps=[time.time()] * 2 + ) + def test_log_value_errors(self): exp = init(mode="debug", flush_period=0.5) img = FileVal.as_image(PIL.Image.new("RGB", (60, 30), color="red")) From 74a6b3f5008bc2e1beb5462f43d9c2ad0f620b0e Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Fri, 18 Nov 2022 22:06:16 +0100 Subject: [PATCH 13/26] Clean code --- src/neptune/new/handler.py | 56 +++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 53c097c97..83d6d5c62 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -339,30 +339,13 @@ def append( """ verify_type("step", step, (int, float, type(None))) verify_type("timestamp", timestamp, (int, float, type(None))) - if is_collection(value): - raise NeptuneUserApiInputException("Value cannot be a collection") - if is_dict_like(value): - for element in value.values(): - if is_collection(element): - raise NeptuneUserApiInputException("Dict value cannot be a collection, use extend method") if step is not None: step = [step] if timestamp is not None: timestamp = [timestamp] - value = ExtendUtils.transform_to_extend_format(value) - self._do_extend(value, step, timestamp, wait, **kwargs) - - def _do_extend(self, values, steps, timestamps, wait, **kwargs): - ExtendUtils.validate_values_for_extend(values, steps, timestamps) - - with self._container.lock(): - attr = self._container.get_attribute(self._path) - if not attr: - neptune_value = cast_value_for_extend(values) - attr = ValueToAttributeVisitor(self._container, parse_path(self._path)).visit(neptune_value) - self._container.set_attribute(self._path, attr) - attr.extend(values, steps=steps, timestamps=timestamps, wait=wait, **kwargs) + value = ExtendUtils.validate_and_transform_to_extend_format(value) + self.extend(value, step, timestamp, wait, **kwargs) def extend( self, @@ -402,7 +385,16 @@ def extend( ... ts = df["timestamp"] ... run["data/example_series"].extend(ys, timestamps=ts) """ - self._do_extend(values, steps, timestamps, wait, **kwargs) + ExtendUtils.validate_values_for_extend(values, steps, timestamps) + + with self._container.lock(): + attr = self._container.get_attribute(self._path) + if not attr: + neptune_value = cast_value_for_extend(values) + attr = ValueToAttributeVisitor(self._container, parse_path(self._path)).visit(neptune_value) + self._container.set_attribute(self._path, attr) + + attr.extend(values, steps=steps, timestamps=timestamps, wait=wait, **kwargs) @check_protected_paths def add(self, values: Union[str, Iterable[str]], wait: bool = False) -> None: @@ -646,18 +638,24 @@ def __delitem__(self, path) -> None: class ExtendUtils: @staticmethod - def transform_to_extend_format(value): + def validate_and_transform_to_extend_format(value): """Preserve nested structure created by `Namespaces` and `dict_like` objects, but replace all other values with single-element lists, - so work can be delegated to `_do_extend` method.""" - if not isinstance(value, Namespace) and not is_dict_like(value): - return [value] + so work can be delegated to `extend` method.""" + if isinstance(value, Namespace) or is_dict_like(value): + return {k: ExtendUtils.validate_and_transform_to_extend_format(v) for k, v in value.items()} + elif is_collection(value): + raise NeptuneUserApiInputException( + "Value cannot be a collection, if you want to `append` multiple values at once use `extend` method." + ) else: - return {k: ExtendUtils.transform_to_extend_format(v) for k, v in value.items()} + return [value] @staticmethod def validate_values_for_extend(values, steps, timestamps): - collections_lengths = list(ExtendUtils.generate_leaf_collection_lengths(values)) + """Validates if input data is a collection or Namespace with collections leafs. + If steps or timestamps are passed, check if its length is equal to all given values.""" + collections_lengths = set(ExtendUtils.generate_leaf_collection_lengths(values)) if len(collections_lengths) > 1: if steps is not None: @@ -679,8 +677,4 @@ def generate_leaf_collection_lengths(values) -> Iterator[int]: elif is_collection(values): yield len(values) else: - raise NeptuneUserApiInputException( - "Value must be a collection or dict of collections." - " To append multiple values at once, use extend() instead:" - " run['series'].extend(collection)" - ) + raise NeptuneUserApiInputException("Values must be a collection or Namespace leafs must be collections") From 256e54354c6237b07ca549b4d4387380c836f735 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Fri, 18 Nov 2022 22:42:04 +0100 Subject: [PATCH 14/26] Add ExtendDictT --- src/neptune/new/handler.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 83d6d5c62..6a0aaabda 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -17,6 +17,7 @@ from typing import ( TYPE_CHECKING, Any, + Collection, Dict, Iterable, Iterator, @@ -85,6 +86,9 @@ def inner_fun(self: "Handler", *args, **kwargs): return inner_fun +ExtendDictT = Union[Collection[Any], Dict[Any, "ExtendDictT"]] + + class Handler: # paths which can't be modified by client directly _PROTECTED_PATHS = { @@ -349,9 +353,9 @@ def append( def extend( self, - values: Union[Dict[str, Iterable[Any]], Iterable[Any]], - steps: Optional[Iterable[float]] = None, - timestamps: Optional[Iterable[float]] = None, + values: ExtendDictT, + steps: Optional[Collection[float]] = None, + timestamps: Optional[Collection[float]] = None, wait: bool = False, **kwargs, ) -> None: From b6c5873805c5915198eb94e875117c1c709eb89c Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Mon, 21 Nov 2022 12:00:27 +0100 Subject: [PATCH 15/26] Expect key in nested dicts to be str --- src/neptune/new/handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 6a0aaabda..3d8261e2a 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -86,7 +86,7 @@ def inner_fun(self: "Handler", *args, **kwargs): return inner_fun -ExtendDictT = Union[Collection[Any], Dict[Any, "ExtendDictT"]] +ExtendDictT = Union[Collection[Any], Dict[str, "ExtendDictT"]] class Handler: From ec8a6b745cf8d4c721ee18c69bf14737927c0a77 Mon Sep 17 00:00:00 2001 From: jkushner Date: Wed, 30 Nov 2022 12:33:11 +0100 Subject: [PATCH 16/26] PR fixes --- src/neptune/new/attributes/namespace.py | 3 +-- src/neptune/new/attributes/series/series.py | 12 ++++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/neptune/new/attributes/namespace.py b/src/neptune/new/attributes/namespace.py index 52affa798..856e89fe7 100644 --- a/src/neptune/new/attributes/namespace.py +++ b/src/neptune/new/attributes/namespace.py @@ -29,7 +29,6 @@ ) from neptune.new.attributes.attribute import Attribute -from neptune.new.attributes.series.series import DataTV from neptune.new.internal.container_structure import ContainerStructure from neptune.new.internal.utils.generic_attribute_mapper import ( NoValue, @@ -70,7 +69,7 @@ def __iter__(self) -> Iterator[str]: def extend( self, - value: Union[DataTV, Iterable[DataTV]], + value: Union[Any, Iterable[Any]], steps: Optional[Collection[float]] = None, timestamps: Optional[Collection[float]] = None, wait: bool = False, diff --git a/src/neptune/new/attributes/series/series.py b/src/neptune/new/attributes/series/series.py index dad62ba3b..3cdb46448 100644 --- a/src/neptune/new/attributes/series/series.py +++ b/src/neptune/new/attributes/series/series.py @@ -53,8 +53,12 @@ def _get_log_operations_from_value( ) -> List[LogOperationTV]: if steps is None: steps = cycle([None]) + else: + assert len(value) == len(steps) if timestamps is None: timestamps = cycle([time.time()]) + else: + assert len(value) == len(timestamps) mapped_values = self._map_series_val(value) values_with_step_and_ts = zip(mapped_values, steps, timestamps) @@ -140,10 +144,14 @@ def extend( if steps is not None: verify_collection_type("steps", steps, (float, int)) - assert len(steps) == len(values) + if len(steps) != len(values): + raise ValueError(f"Number of steps must be equal to number of values ({len(steps)} != {len(values)}") if timestamps is not None: verify_collection_type("timestamps", timestamps, (float, int)) - assert len(timestamps) == len(values) + if len(timestamps) != len(values): + raise ValueError( + f"Number of timestamps must be equal to number of values ({len(timestamps)} != {len(values)}" + ) ops = self._get_log_operations_from_value(value, steps=steps, timestamps=timestamps) From 1ba405e0f75b5d959111290cb69739a3dd9eb6ef Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Thu, 1 Dec 2022 12:00:40 +0100 Subject: [PATCH 17/26] CR fixes --- src/neptune/new/attributes/series/series.py | 2 +- .../new/attributes/series/string_series.py | 25 ++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/neptune/new/attributes/series/series.py b/src/neptune/new/attributes/series/series.py index 3cdb46448..4d63dab06 100644 --- a/src/neptune/new/attributes/series/series.py +++ b/src/neptune/new/attributes/series/series.py @@ -69,7 +69,7 @@ def _get_log_operations_from_value( @classmethod def _map_series_val(cls, value: ValTV) -> List[DataTV]: - return [value for value in value.values] + return value.values def _get_config_operation_from_value(self, value: ValTV) -> Optional[LogOperationTV]: return None diff --git a/src/neptune/new/attributes/series/string_series.py b/src/neptune/new/attributes/series/string_series.py index 518431d4a..0a369bfcd 100644 --- a/src/neptune/new/attributes/series/string_series.py +++ b/src/neptune/new/attributes/series/string_series.py @@ -15,8 +15,10 @@ # from typing import ( TYPE_CHECKING, + Collection, Iterable, List, + Union, ) from neptune.new.attributes.series.fetchable_series import FetchableSeries @@ -48,14 +50,9 @@ def __init__(self, container: "MetadataContainer", path: List[str]): super().__init__(container, path) self._value_truncation_occurred = False - def _get_clear_operation(self) -> Operation: - return ClearStringLog(self._path) - - def _data_to_value(self, values: Iterable, **kwargs) -> Val: - if kwargs: - logger.warning("Warning: unexpected arguments (%s) in StringSeries", kwargs) - - value = StringSeriesVal(values) + def _get_log_operations_from_value( + self, value: Val, *, steps: Union[None, Collection[float]], timestamps: Union[None, Collection[float]] + ) -> List[LogOperation]: if not self._value_truncation_occurred and value.truncated: # the first truncation self._value_truncation_occurred = True @@ -66,7 +63,17 @@ def _data_to_value(self, values: Iterable, **kwargs) -> Val: path_to_str(self._path), MAX_STRING_SERIES_VALUE_LENGTH, ) - return value + + return super()._get_log_operations_from_value(value, steps=steps, timestamps=timestamps) + + def _get_clear_operation(self) -> Operation: + return ClearStringLog(self._path) + + def _data_to_value(self, values: Iterable, **kwargs) -> Val: + if kwargs: + logger.warning("Warning: unexpected arguments (%s) in StringSeries", kwargs) + + return StringSeriesVal(values) def _is_value_type(self, value) -> bool: return isinstance(value, StringSeriesVal) From c311b29da2dae3cc71da7b6fe8152290dc609d1f Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Thu, 1 Dec 2022 21:05:38 +0100 Subject: [PATCH 18/26] CR fixes --- src/neptune/new/handler.py | 15 +++++---------- .../new/metadata_containers/metadata_container.py | 8 +++++++- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 3d8261e2a..f1f32cd24 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -65,7 +65,6 @@ cast_value, cast_value_for_extend, ) -from neptune.new.types.value_copy import ValueCopy if TYPE_CHECKING: from neptune.new.metadata_containers import MetadataContainer @@ -171,14 +170,11 @@ def assign(self, value, wait: bool = False) -> None: with self._container.lock(): attr = self._container.get_attribute(self._path) if not attr: + self._container.define(self._path, value) + else: + # TODO: we should clean some rules regarding type casting in assignment neptune_value = cast_value(value) - attr = ValueToAttributeVisitor(self._container, parse_path(self._path)).visit(neptune_value) - self._container.set_attribute(self._path, attr) - value = neptune_value # TODO: we should clean some rules regarding type casting in assignment - - if isinstance(value, Handler): - value = ValueCopy(value) - attr.process_assignment(value, wait) + attr.process_assignment(neptune_value, wait) @check_protected_paths def upload(self, value, wait: bool = False) -> None: @@ -631,8 +627,7 @@ def track_files(self, path: str, destination: str = None, wait: bool = False) -> attr = self._container.get_attribute(self._path) if not attr: attr = Artifact(self._container, parse_path(self._path)) - - self._container.set_attribute(self._path, attr) + self._container.set_attribute(self._path, attr) attr.track_files(path=path, destination=destination, wait=wait) diff --git a/src/neptune/new/metadata_containers/metadata_container.py b/src/neptune/new/metadata_containers/metadata_container.py index 4b209b518..0dc966ef7 100644 --- a/src/neptune/new/metadata_containers/metadata_container.py +++ b/src/neptune/new/metadata_containers/metadata_container.py @@ -65,8 +65,10 @@ in_notebook, ) from neptune.new.internal.utils.uncaught_exception_handler import instance as uncaught_exception_handler +from neptune.new.internal.value_to_attribute_visitor import ValueToAttributeVisitor from neptune.new.metadata_containers.metadata_containers_table import Table from neptune.new.types.mode import Mode +from neptune.new.types.type_casting import cast_value def ensure_not_stopped(fun): @@ -240,7 +242,11 @@ def define( if old_attr is not None: raise MetadataInconsistency("Attribute or namespace {} is already defined".format(path)) - self[path].assign(value, wait=wait) + neptune_value = cast_value(value) + attr = ValueToAttributeVisitor(self, parse_path(path)).visit(neptune_value) + self.set_attribute(path, attr) + attr.process_assignment(neptune_value, wait) + return attr def get_attribute(self, path: str) -> Optional[Attribute]: with self._lock: From 5ea30ed5795c709383446c8f828289981e46a7a4 Mon Sep 17 00:00:00 2001 From: jkushner Date: Mon, 5 Dec 2022 15:06:34 +0100 Subject: [PATCH 19/26] Post review fixes --- src/neptune/new/attributes/series/file_series.py | 5 ----- src/neptune/new/handler.py | 1 - 2 files changed, 6 deletions(-) diff --git a/src/neptune/new/attributes/series/file_series.py b/src/neptune/new/attributes/series/file_series.py index 0e9e60fb1..af12da0b1 100644 --- a/src/neptune/new/attributes/series/file_series.py +++ b/src/neptune/new/attributes/series/file_series.py @@ -68,11 +68,6 @@ def _data_to_value(self, values: Iterable, **kwargs) -> Val: def _is_value_type(self, value) -> bool: return isinstance(value, FileSeriesVal) - @classmethod - def _map_log_value(cls, value): - # TODO: maybe should be moved to operations - return cls._get_base64_image_content(value) - @staticmethod def _get_base64_image_content(file: File) -> str: if file.file_type is FileType.LOCAL_FILE: diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index f1f32cd24..6fbb89a0b 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -172,7 +172,6 @@ def assign(self, value, wait: bool = False) -> None: if not attr: self._container.define(self._path, value) else: - # TODO: we should clean some rules regarding type casting in assignment neptune_value = cast_value(value) attr.process_assignment(neptune_value, wait) From 558c70bc401bc37231332687a4d4566d02751e84 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Mon, 5 Dec 2022 17:03:32 +0100 Subject: [PATCH 20/26] Protect Series attributes overriden --- src/neptune/new/attributes/series/series.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/neptune/new/attributes/series/series.py b/src/neptune/new/attributes/series/series.py index 4d63dab06..5023806c2 100644 --- a/src/neptune/new/attributes/series/series.py +++ b/src/neptune/new/attributes/series/series.py @@ -45,6 +45,14 @@ class Series(Attribute, Generic[ValTV, DataTV, LogOperationTV]): MAX_BATCH_SIZE = None operation_cls: type(LogOperationTV) = None + def __init_subclass__(cls): + required_attributes = ("MAX_BATCH_SIZE", "operation_cls") + for required_attribute in required_attributes: + if not any(required_attribute in base.__dict__ for base in cls.__mro__ if base is not Series): + raise NotImplementedError( + f"Attribute '{required_attribute}' has not been overwritten in class '{cls.__name__}'" + ) + def clear(self, wait: bool = False) -> None: self._clear_impl(wait) From 6280108a12f9fd11c330b9a47723873f9725bc44 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Tue, 6 Dec 2022 11:19:39 +0100 Subject: [PATCH 21/26] Use __init_subclass__ attributes --- src/neptune/new/attributes/series/file_series.py | 5 +---- src/neptune/new/attributes/series/float_series.py | 7 +++---- src/neptune/new/attributes/series/series.py | 15 ++++----------- .../new/attributes/series/string_series.py | 7 +++---- 4 files changed, 11 insertions(+), 23 deletions(-) diff --git a/src/neptune/new/attributes/series/file_series.py b/src/neptune/new/attributes/series/file_series.py index af12da0b1..14c2b851e 100644 --- a/src/neptune/new/attributes/series/file_series.py +++ b/src/neptune/new/attributes/series/file_series.py @@ -44,10 +44,7 @@ LogOperation = LogImages -class FileSeries(Series[Val, Data, LogOperation]): - MAX_BATCH_SIZE = 1 - operation_cls = LogOperation - +class FileSeries(Series[Val, Data, LogOperation], max_batch_size=100, operation_cls=LogOperation): @classmethod def _map_series_val(cls, value: Val) -> List[ImageValue]: return [ diff --git a/src/neptune/new/attributes/series/float_series.py b/src/neptune/new/attributes/series/float_series.py index 59517465e..22660ad36 100644 --- a/src/neptune/new/attributes/series/float_series.py +++ b/src/neptune/new/attributes/series/float_series.py @@ -37,10 +37,9 @@ LogOperation = LogFloats -class FloatSeries(Series[Val, Data, LogOperation], FetchableSeries[FloatSeriesValues]): - MAX_BATCH_SIZE = 100 - operation_cls = LogOperation - +class FloatSeries( + Series[Val, Data, LogOperation], FetchableSeries[FloatSeriesValues], max_batch_size=100, operation_cls=LogOperation +): def configure( self, min: Optional[Union[float, int]] = None, diff --git a/src/neptune/new/attributes/series/series.py b/src/neptune/new/attributes/series/series.py index 5023806c2..f8d83cb04 100644 --- a/src/neptune/new/attributes/series/series.py +++ b/src/neptune/new/attributes/series/series.py @@ -42,16 +42,9 @@ class Series(Attribute, Generic[ValTV, DataTV, LogOperationTV]): - MAX_BATCH_SIZE = None - operation_cls: type(LogOperationTV) = None - - def __init_subclass__(cls): - required_attributes = ("MAX_BATCH_SIZE", "operation_cls") - for required_attribute in required_attributes: - if not any(required_attribute in base.__dict__ for base in cls.__mro__ if base is not Series): - raise NotImplementedError( - f"Attribute '{required_attribute}' has not been overwritten in class '{cls.__name__}'" - ) + def __init_subclass__(cls, max_batch_size: int, operation_cls: type(LogOperationTV)): + cls.max_batch_size = max_batch_size + cls.operation_cls = operation_cls def clear(self, wait: bool = False) -> None: self._clear_impl(wait) @@ -72,7 +65,7 @@ def _get_log_operations_from_value( values_with_step_and_ts = zip(mapped_values, steps, timestamps) log_values = [self.operation_cls.ValueType(val, step=step, ts=ts) for val, step, ts in values_with_step_and_ts] return [ - self.operation_cls(self._path, chunk) for chunk in get_batches(log_values, batch_size=self.MAX_BATCH_SIZE) + self.operation_cls(self._path, chunk) for chunk in get_batches(log_values, batch_size=self.max_batch_size) ] @classmethod diff --git a/src/neptune/new/attributes/series/string_series.py b/src/neptune/new/attributes/series/string_series.py index 0a369bfcd..a50f8faa4 100644 --- a/src/neptune/new/attributes/series/string_series.py +++ b/src/neptune/new/attributes/series/string_series.py @@ -42,10 +42,9 @@ LogOperation = LogStrings -class StringSeries(Series[Val, Data, LogOperation], FetchableSeries[StringSeriesValues]): - MAX_BATCH_SIZE = 10 - operation_cls = LogOperation - +class StringSeries( + Series[Val, Data, LogOperation], FetchableSeries[StringSeriesValues], max_batch_size=100, operation_cls=LogOperation +): def __init__(self, container: "MetadataContainer", path: List[str]): super().__init__(container, path) self._value_truncation_occurred = False From 6ec7434f6575dd3e56f1307c27c1aa52c7109b14 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Tue, 6 Dec 2022 11:55:35 +0100 Subject: [PATCH 22/26] Fix tests --- src/neptune/new/handler.py | 11 +++++------ .../neptune/new/attributes/series/test_file_series.py | 8 ++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 6fbb89a0b..0f89ab93c 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -61,10 +61,8 @@ ) from neptune.new.internal.value_to_attribute_visitor import ValueToAttributeVisitor from neptune.new.types.atoms.file import File as FileVal -from neptune.new.types.type_casting import ( - cast_value, - cast_value_for_extend, -) +from neptune.new.types.type_casting import cast_value_for_extend +from neptune.new.types.value_copy import ValueCopy if TYPE_CHECKING: from neptune.new.metadata_containers import MetadataContainer @@ -172,8 +170,9 @@ def assign(self, value, wait: bool = False) -> None: if not attr: self._container.define(self._path, value) else: - neptune_value = cast_value(value) - attr.process_assignment(neptune_value, wait) + if isinstance(value, Handler): + value = ValueCopy(value) + attr.process_assignment(value, wait) @check_protected_paths def upload(self, value, wait: bool = False) -> None: diff --git a/tests/unit/neptune/new/attributes/series/test_file_series.py b/tests/unit/neptune/new/attributes/series/test_file_series.py index f400d99a2..ccfc3f8e1 100644 --- a/tests/unit/neptune/new/attributes/series/test_file_series.py +++ b/tests/unit/neptune/new/attributes/series/test_file_series.py @@ -148,7 +148,7 @@ def test_log_path(self): ) # then - def generate_expected_call(wait, step): + def generate_expected_call(wait, step, values_no=1): log_operation = LogImages( path=path, values=[ @@ -157,7 +157,8 @@ def generate_expected_call(wait, step): step=step, ts=self._now(), ) - ], + ] + * values_no, ) return call( log_operation, @@ -167,8 +168,7 @@ def generate_expected_call(wait, step): op_processor.enqueue_operation.assert_has_calls( [ generate_expected_call(wait, step=3), - generate_expected_call(wait, step=None), - generate_expected_call(wait, step=None), + generate_expected_call(wait, step=None, values_no=2), ] ) From 2027c8ea3b968edf94a856836b7b8ef461442312 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Tue, 6 Dec 2022 12:15:25 +0100 Subject: [PATCH 23/26] Fix copy-paste --- src/neptune/new/attributes/series/file_series.py | 2 +- src/neptune/new/attributes/series/string_series.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/neptune/new/attributes/series/file_series.py b/src/neptune/new/attributes/series/file_series.py index 14c2b851e..e1d20e579 100644 --- a/src/neptune/new/attributes/series/file_series.py +++ b/src/neptune/new/attributes/series/file_series.py @@ -44,7 +44,7 @@ LogOperation = LogImages -class FileSeries(Series[Val, Data, LogOperation], max_batch_size=100, operation_cls=LogOperation): +class FileSeries(Series[Val, Data, LogOperation], max_batch_size=1, operation_cls=LogOperation): @classmethod def _map_series_val(cls, value: Val) -> List[ImageValue]: return [ diff --git a/src/neptune/new/attributes/series/string_series.py b/src/neptune/new/attributes/series/string_series.py index a50f8faa4..b81861e83 100644 --- a/src/neptune/new/attributes/series/string_series.py +++ b/src/neptune/new/attributes/series/string_series.py @@ -43,7 +43,7 @@ class StringSeries( - Series[Val, Data, LogOperation], FetchableSeries[StringSeriesValues], max_batch_size=100, operation_cls=LogOperation + Series[Val, Data, LogOperation], FetchableSeries[StringSeriesValues], max_batch_size=10, operation_cls=LogOperation ): def __init__(self, container: "MetadataContainer", path: List[str]): super().__init__(container, path) From 3c23b404a5f921f813c5ee6bf5f8e6832efde6f9 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Tue, 6 Dec 2022 12:36:39 +0100 Subject: [PATCH 24/26] PR fixes --- src/neptune/new/handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index 0f89ab93c..f0efb260a 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -171,8 +171,8 @@ def assign(self, value, wait: bool = False) -> None: self._container.define(self._path, value) else: if isinstance(value, Handler): - value = ValueCopy(value) - attr.process_assignment(value, wait) + cast_value = ValueCopy(value) + attr.process_assignment(cast_value, wait) @check_protected_paths def upload(self, value, wait: bool = False) -> None: From d1db6a81bf7267ecd2b35eee2d11a4af95fc06f9 Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Tue, 6 Dec 2022 12:47:26 +0100 Subject: [PATCH 25/26] PR fixes --- src/neptune/new/handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/neptune/new/handler.py b/src/neptune/new/handler.py index f0efb260a..0f89ab93c 100644 --- a/src/neptune/new/handler.py +++ b/src/neptune/new/handler.py @@ -171,8 +171,8 @@ def assign(self, value, wait: bool = False) -> None: self._container.define(self._path, value) else: if isinstance(value, Handler): - cast_value = ValueCopy(value) - attr.process_assignment(cast_value, wait) + value = ValueCopy(value) + attr.process_assignment(value, wait) @check_protected_paths def upload(self, value, wait: bool = False) -> None: From e7732fd45c6731ee1ac8f61b811ed72f52fa21bb Mon Sep 17 00:00:00 2001 From: Jakub Kuszneruk Date: Tue, 6 Dec 2022 13:42:14 +0100 Subject: [PATCH 26/26] Fix tests --- .../neptune/new/attributes/series/test_file_series.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/neptune/new/attributes/series/test_file_series.py b/tests/unit/neptune/new/attributes/series/test_file_series.py index ccfc3f8e1..f400d99a2 100644 --- a/tests/unit/neptune/new/attributes/series/test_file_series.py +++ b/tests/unit/neptune/new/attributes/series/test_file_series.py @@ -148,7 +148,7 @@ def test_log_path(self): ) # then - def generate_expected_call(wait, step, values_no=1): + def generate_expected_call(wait, step): log_operation = LogImages( path=path, values=[ @@ -157,8 +157,7 @@ def generate_expected_call(wait, step, values_no=1): step=step, ts=self._now(), ) - ] - * values_no, + ], ) return call( log_operation, @@ -168,7 +167,8 @@ def generate_expected_call(wait, step, values_no=1): op_processor.enqueue_operation.assert_has_calls( [ generate_expected_call(wait, step=3), - generate_expected_call(wait, step=None, values_no=2), + generate_expected_call(wait, step=None), + generate_expected_call(wait, step=None), ] )