Repository: yukinarit/pyserde (python)

Issue:
`skip_none` is a no-op in JSON, YAML, and Pickle
`serde.json.to_json`, `serde.yaml.to_yaml`, and `serde.pickle.to_pickle` are all documented as – and accept – a `skip_none` argument. However, it's a no-op, as it's dropped on the floor instead of being passed to `as_dict`. It seems like only TOML fully implemented it. (And interestingly, MsgPack isn't documented as accepting that option.)

Compare [YAML](https://github.com/yukinarit/pyserde/blob/1e3b0698e81b6498f5360e4c56cc8d14cc477a74/serde/yaml.py#L51) with [TOML](https://github.com/yukinarit/pyserde/blob/1e3b0698e81b6498f5360e4c56cc8d14cc477a74/serde/toml.py#L64).

I don't mind to try and contribute a fix, but wanted to be sure it would be welcome before I spend the time. Thanks!

Pull request:
Fix skip_none parameter in several serialization modules
Fixes #645 by passing `skip_none` to YAML/JSON serializers. The changes are demonstrated by new tests that fail without my change and pass with my change.

~~For Pickle, the argument didn't seem to make a difference, so I marked it as deprecated. No tests are introduced.~~ *Edit: this change adds multiple lines of "untested code" according to Codecov, and introducing tests just to prove it raises a warning seems pointless. I dropped this change.*

I also noticed that one of the TOML tests wasn't being called because its name didn't start with `test_`. And the second half of the test case wasn't asserting anything. I fixed that test module to match with the ones I introduced for YAML/JSON.

Patch:
diff --git a/serde/json.py b/serde/json.py
index a664e3b..52eadea 100644
--- a/serde/json.py
+++ b/serde/json.py
@@ -76,7 +76,14 @@ def to_json(
     your own logic.
     """
     return se.serialize(
-        to_dict(obj, c=cls, reuse_instances=reuse_instances, convert_sets=convert_sets), **opts
+        to_dict(
+            obj,
+            c=cls,
+            reuse_instances=reuse_instances,
+            convert_sets=convert_sets,
+            skip_none=skip_none,
+        ),
+        **opts,
     )
 
 
diff --git a/serde/yaml.py b/serde/yaml.py
index f0b90a6..4618a48 100644
--- a/serde/yaml.py
+++ b/serde/yaml.py
@@ -48,7 +48,14 @@ def to_yaml(
     your own logic.
     """
     return se.serialize(
-        to_dict(obj, c=cls, reuse_instances=reuse_instances, convert_sets=convert_sets), **opts
+        to_dict(
+            obj,
+            c=cls,
+            reuse_instances=reuse_instances,
+            convert_sets=convert_sets,
+            skip_none=skip_none,
+        ),
+        **opts,
     )
 
 


Test patch:
diff --git a/tests/test_json.py b/tests/test_json.py
new file mode 100644
index 0000000..20d1be6
--- /dev/null
+++ b/tests/test_json.py
@@ -0,0 +1,34 @@
+from serde import serde
+from serde.json import to_json, from_json
+from typing import Optional
+
+
+def test_json_basics() -> None:
+    @serde
+    class Foo:
+        v: Optional[int]
+
+    f = Foo(10)
+    assert '{"v":10}' == to_json(f)
+    assert f == from_json(Foo, '{"v":10}')
+
+    @serde
+    class Bar:
+        v: set[int]
+
+    b = Bar({1, 2, 3})
+    assert '{"v":[1,2,3]}' == to_json(b)
+    assert b == from_json(Bar, '{"v":[1,2,3]}')
+
+
+def test_skip_none() -> None:
+    @serde
+    class Foo:
+        a: int
+        b: Optional[int]
+
+    f = Foo(10, 100)
+    assert to_json(f, skip_none=True) == '{"a":10,"b":100}'
+
+    f = Foo(10, None)
+    assert to_json(f, skip_none=True) == '{"a":10}'
diff --git a/tests/test_toml.py b/tests/test_toml.py
index 414af18..969cb16 100644
--- a/tests/test_toml.py
+++ b/tests/test_toml.py
@@ -4,7 +4,7 @@ from typing import Optional
 import pytest
 
 
-def toml_basics() -> None:
+def test_toml_basics() -> None:
     @serde
     class Foo:
         v: Optional[int]
@@ -17,8 +17,16 @@ def toml_basics() -> None:
     class Bar:
         v: set[int]
 
+    toml_literal = """\
+v = [
+    1,
+    2,
+    3,
+]
+"""
     b = Bar({1, 2, 3})
-    to_toml(b)
+    assert toml_literal == to_toml(b)
+    assert b == from_toml(Bar, toml_literal)
 
 
 def test_skip_none() -> None:
diff --git a/tests/test_yaml.py b/tests/test_yaml.py
new file mode 100644
index 0000000..a4ff770
--- /dev/null
+++ b/tests/test_yaml.py
@@ -0,0 +1,45 @@
+from serde import serde
+from serde.yaml import to_yaml, from_yaml
+from typing import Optional
+
+
+def test_yaml_basics() -> None:
+    @serde
+    class Foo:
+        v: Optional[int]
+
+    f = Foo(10)
+    assert "v: 10\n" == to_yaml(f)
+    assert f == from_yaml(Foo, "v: 10\n")
+
+    @serde
+    class Bar:
+        v: set[int]
+
+    b = Bar({1, 2, 3})
+    assert "v:\n- 1\n- 2\n- 3\n" == to_yaml(b)
+    assert b == from_yaml(Bar, "v:\n- 1\n- 2\n- 3\n")
+
+
+def test_skip_none() -> None:
+    @serde
+    class Foo:
+        a: int
+        b: Optional[int]
+
+    f = Foo(10, 100)
+    assert (
+        to_yaml(f, skip_none=True)
+        == """\
+a: 10
+b: 100
+"""
+    )
+
+    f = Foo(10, None)
+    assert (
+        to_yaml(f, skip_none=True)
+        == """\
+a: 10
+"""
+    )