Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabled automatic casting to strings #1215

Merged
merged 7 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## [UNRELEASED] neptune-client 1.0.0

### Changes
- Disabled automatic casting to strings for unsupported by Neptune types ([#1215](https://github.com/neptune-ai/neptune-client/pull/1215))
- Moved modules from `neptune.new` to `neptune` with compatibility imports and marked `neptune.new` as deprecated ([#1213](https://github.com/neptune-ai/neptune-client/pull/1213))
- Removed `neptune.*` legacy modules ([#1206](https://github.com/neptune-ai/neptune-client/pull/1206))

Expand Down
15 changes: 1 addition & 14 deletions src/neptune/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@
from neptune.attributes.series.float_series import FloatSeries
from neptune.attributes.series.string_series import StringSeries
from neptune.attributes.sets.string_set import StringSet
from neptune.common.deprecation import warn_once

# backwards compatibility
from neptune.common.exceptions import NeptuneException # noqa: F401
from neptune.exceptions import (
MissingFieldException,
NeptuneCannotChangeStageManually,
Expand All @@ -54,7 +50,6 @@
is_float,
is_float_like,
is_string,
is_string_like,
is_stringify_value,
verify_collection_type,
verify_type,
Expand Down Expand Up @@ -329,15 +324,7 @@ def log(
attr = FileSeries(self._container, parse_path(self._path))
elif is_float_like(first_value):
attr = FloatSeries(self._container, parse_path(self._path))
elif is_string_like(first_value):
if not from_stringify_value:
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))` or"
" `.log(stringify_unsupported(collection))` for collections and dictionaries."
" For details, see https://docs.neptune.ai/setup/neptune-client_1-0_release_changes"
)
elif from_stringify_value:
attr = StringSeries(self._container, parse_path(self._path))
else:
raise TypeError("Value of unsupported type {}".format(type(first_value)))
Expand Down
2 changes: 1 addition & 1 deletion src/neptune/integrations/python_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def __init__(self, *, run: Run, level=logging.NOTSET, path: str = None):
self._logger = Logger(run, path)
self._thread_local = threading.local()

self._run[INTEGRATION_VERSION_KEY] = neptune_client_version
self._run[INTEGRATION_VERSION_KEY] = str(neptune_client_version)

def emit(self, record: logging.LogRecord) -> None:
if not hasattr(self._thread_local, "inside_write"):
Expand Down
22 changes: 8 additions & 14 deletions src/neptune/types/atoms/string.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@
from dataclasses import dataclass
from typing import (
TYPE_CHECKING,
Optional,
TypeVar,
Union,
)

from neptune.common.deprecation import warn_once
from neptune.internal.utils import (
is_string,
is_stringify_value,
verify_type,
)
from neptune.internal.utils.stringify_value import StringifyValue
from neptune.types.atoms.atom import Atom

if TYPE_CHECKING:
Expand All @@ -39,21 +41,13 @@ class String(Atom):

value: str

def __init__(self, value):
if is_stringify_value(value):
value = str(value.value)
def __init__(self, value: Optional[Union[str, StringifyValue]]):
verify_type("value", value, (str, type(None), StringifyValue))

if not is_string(value):
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 `String(str(object))` or `String(repr(object))` instead."
)

self.value = str(value)
self.value = str(value.value) if is_stringify_value(value) else value

def accept(self, visitor: "ValueVisitor[Ret]") -> Ret:
return visitor.visit_string(self)

def __str__(self):
def __str__(self) -> str:
return "String({})".format(str(self.value))
5 changes: 1 addition & 4 deletions src/neptune/types/series/string_series.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@


def extract_value(value: Any) -> str:
if is_stringify_value(value):
value = str(value.value)

return str(value)
return str(value.value) if is_stringify_value(value) else value


class StringSeries(Series):
Expand Down
22 changes: 3 additions & 19 deletions src/neptune/types/type_casting.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,14 @@
Union,
)

from neptune.common.deprecation import warn_once
from neptune.internal.utils import (
is_bool,
is_collection,
is_dict_like,
is_float,
is_float_like,
is_int,
is_string,
is_string_like,
is_stringify_value,
)
from neptune.types import (
Expand Down Expand Up @@ -84,14 +83,7 @@ def cast_value(value: Any) -> Value:
return Float(value)
elif is_dict_like(value):
return Namespace(value)
elif is_string_like(value):
if not from_stringify_value:
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)` or `repr(object)` instead."
" For details, see https://docs.neptune.ai/setup/neptune-client_1-0_release_changes"
)
elif (is_collection(value) and all(is_stringify_value(elem) for elem in value)) or from_stringify_value:
return String(str(value))
else:
raise TypeError("Value of unsupported type {}".format(type(value)))
Expand Down Expand Up @@ -121,15 +113,7 @@ def cast_value_for_extend(values: Union[Namespace, Series, Collection[Any]]) ->
return StringSeries(values=values)
elif is_float_like(sample_val):
return FloatSeries(values=values)
elif is_string_like(sample_val):
if not from_stringify_value:
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)` or"
" `stringify_unsupported(collection)` for collections and dictionaries."
" For details, see https://docs.neptune.ai/setup/neptune-client_1-0_release_changes"
)
elif from_stringify_value:
return StringSeries(values=values)
else:
raise TypeError("Value of unsupported type List[{}]".format(type(sample_val)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@normandy7 after 1.0.0 this will raise TypeError if anyone will try to assign an unsupported type (including all that were converted to string in 0.x.x and anything else that were unsupported in prior versions as well). Maybe we would like to improve it anyhow? This also applies to log and extend & append methods and could be specialized to the called method (append and extend are combined).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Raalsky Yes, I think we can use a similar message as before. I think it might be best to create a docs page for this error and how to work around it, then we can simply add a link.

Would something like this work?

            warn_once(
                message="The type of the object you're logging is not supported by"
                " Neptune. To log the object as a string, use `str(object)` or"
                " `stringify_unsupported(collection)` for collections and dictionaries."
                " For details, see https://docs.neptune.ai/help/value_of_unsupported_type"
            )

I can open a separate PR with this if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This couldn't be a warning. Probably a new exception. I'll take care of it later today 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 changes: 1 addition & 1 deletion tests/unit/neptune/new/test_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def test_assign_dict(self):
"str": "imagine",
"float": 3.14,
"datetime": now,
"list": list(range(10)),
"list": str(list(range(10))),
},
}
)
Expand Down
82 changes: 3 additions & 79 deletions tests/unit/neptune/new/test_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,22 +220,10 @@ def test_log(self):

def test_log_dict(self):
with init_run(mode="debug", flush_period=0.5) as exp:
dict_value = {"key-a": "value-a", "key-b": "value-b"}
dict_value = str({"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):
with init_run(mode="debug", flush_period=0.5) as exp:
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):
with init_run(mode="debug", flush_period=0.5) as exp:
exp["some/num/val"].append(5)
Expand Down Expand Up @@ -320,29 +308,6 @@ 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_dict_in_list(self):
"""We expect that everything which is inside Collection is mapped to atoms"""
with init_run(mode="debug", flush_period=0.5) as exp:
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}}, # 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}], # 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}}")
self.assertEqual(
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"""
with init_run(mode="debug", flush_period=0.5) as exp:
Expand Down Expand Up @@ -500,7 +465,7 @@ def test_assign_empty_dict(self):
def test_argparse_namespace(self):
with init_run(mode="debug", flush_period=0.5) as exp:
exp["params"] = argparse.Namespace(
foo="bar", baz=42, nested=argparse.Namespace(nested_attr=[1, 2, 3], num=55)
foo="bar", baz=42, nested=argparse.Namespace(nested_attr=str([1, 2, 3]), num=55)
)
self.assertEqual(exp["params/foo"].fetch(), "bar")
self.assertEqual(exp["params/baz"].fetch(), 42)
Expand Down Expand Up @@ -698,34 +663,6 @@ def test_extend_float_like_types(self):
with self.assertRaises(ValueError):
exp["attr"].extend([4, "234a"])

def test_string_like_types(self):
with init_run(mode="debug", flush_period=0.5) as exp:
exp["attr1"] = "234"
exp["attr1"] = self.FloatLike(12356)
self.assertEqual(exp["attr1"].fetch(), "TestOtherBehaviour.FloatLike(value=12356)")

exp["attr2"].log("xxx")
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):
with init_run(mode="debug", flush_period=0.5) as exp:
exp["attr1"] = "234"
exp["attr1"] = self.FloatLike(12356)
self.assertEqual(exp["attr1"].fetch(), "TestOtherBehaviour.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_extend_string_like_types(self):
with init_run(mode="debug", flush_period=0.5) as exp:
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):
with init_run(mode="debug", flush_period=0.5) as exp:
exp["params"] = {
Expand All @@ -743,26 +680,13 @@ def test_assign_dict(self):
def test_convertable_to_dict(self):
with init_run(mode="debug", flush_period=0.5) as exp:
exp["params"] = argparse.Namespace(
foo="bar", baz=42, nested=argparse.Namespace(nested_attr=[1, 2, 3], num=55)
foo="bar", baz=42, nested=argparse.Namespace(nested_attr=str([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):
self.name = name
self.fierceness = fierceness

def __str__(self):
return f"{self.name} goes " + "Woof! " * self.fierceness

with init_run(mode="debug", flush_period=0.5) as exp:
exp["burek"] = Dog("Burek", 3)
self.assertEqual(exp["burek"].fetch(), "Burek goes Woof! Woof! Woof! ")

def test_representation(self):
with init_run(mode="debug", flush_period=0.5) as exp:
exp["params/int"] = 1
Expand Down
Loading