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

Dont dump none entity #31

Closed
wants to merge 18 commits into from
Closed
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ __pycache__/
.coverage
.auto-format
/toml-test/
/.env/
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,26 @@ Parse a TOML string and return a python dictionary. (provided to match the inter

#### dumps
```python
def dumps(obj: Any, *, pretty: bool = False) -> str: ...
def dumps(obj: Any, *, pretty: bool = False, include_none = True) -> str: ...
```

Serialize a python object to TOML.

If `pretty` is true, output has a more "pretty" format.

If `include_none` is false, none objectes are ommited (in dictionaries, tuples and lists as well).
zhiburt marked this conversation as resolved.
Show resolved Hide resolved

#### dump
```python
def dump(obj: Any, file: Union[Path, TextIO], *, pretty: bool = False) -> int: ...
def dump(obj: Any, file: Union[Path, TextIO], *, pretty: bool = False, include_none = True) -> int: ...
```

Serialize a python object to TOML and write it to a file. `file` may be a `Path` or file object from `open()`.

If `pretty` is true, output has a more "pretty" format.

If `include_none` is false, none objectes are ommited (in dictionaries, tuples and lists as well).
zhiburt marked this conversation as resolved.
Show resolved Hide resolved

### Example

```py
Expand Down
8 changes: 4 additions & 4 deletions rtoml/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def loads(toml: str) -> Dict[str, Any]:
return _rtoml.deserialize(toml)


def dumps(obj: Any, *, pretty: bool = False) -> str:
def dumps(obj: Any, *, pretty: bool = False, include_none: bool = True) -> str:
"""
Serialize a python object to TOML.

Expand All @@ -45,16 +45,16 @@ def dumps(obj: Any, *, pretty: bool = False) -> str:
else:
serialize = _rtoml.serialize

return serialize(obj)
return serialize(obj, include_none);
zhiburt marked this conversation as resolved.
Show resolved Hide resolved


def dump(obj: Any, file: Union[Path, TextIO], *, pretty: bool = False) -> int:
def dump(obj: Any, file: Union[Path, TextIO], *, pretty: bool = False, include_none: bool = True) -> int:
"""
Serialize a python object to TOML and write it to a file. `file` may be a `Path` or file object from `open()`.

If `pretty` is true, output has a more "pretty" format.
"""
s = dumps(obj, pretty=pretty)
s = dumps(obj, pretty=pretty, include_none=include_none)
if isinstance(file, Path):
return file.write_text(s, encoding='UTF-8')
else:
Expand Down
4 changes: 2 additions & 2 deletions rtoml/_rtoml.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ from typing import Any, Callable, Union
VERSION: str

def deserialize(toml: str) -> Any: ...
def serialize(obj: Any) -> str: ...
def serialize_pretty(obj: Any) -> str: ...
def serialize(obj: Any, include_none: bool) -> str: ...
def serialize_pretty(obj: Any, include_none: bool) -> str: ...

class TomlParsingError(ValueError): ...
class TomlSerializationError(ValueError): ...
79 changes: 47 additions & 32 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ extern crate pyo3;

use pyo3::exceptions::PyValueError;
use pyo3::prelude::*;
use pyo3::types::{PyAny, PyDateTime, PyDict, PyFloat, PyList, PyTuple};
use pyo3::types::{PyAny, PyBool, PyDateTime, PyDict, PyFloat, PyList, PyTuple};
use pyo3::{create_exception, wrap_pyfunction, PyErrArguments};
use serde::ser::{self, Serialize, SerializeMap, SerializeSeq, Serializer};
use std::str::FromStr;
Expand Down Expand Up @@ -49,12 +49,22 @@ fn deserialize(py: Python, toml: String) -> PyResult<PyObject> {
}

// taken from https://github.com/mre/hyperjson/blob/10d31608584ef4499d6b6b10b6dc9455b358fe3d/src/lib.rs#L287-L402
struct SerializePyObject<'p, 'a> {
struct SerializePyObject<'p> {
py: Python<'p>,
obj: &'a PyAny,
obj: &'p PyAny,
include_none: bool,
}

impl<'p, 'a> Serialize for SerializePyObject<'p, 'a> {
impl<'p> SerializePyObject<'p> {
fn new(py: Python<'p>, obj: &'p PyObject, include_none: &PyObject) -> PyResult<Self> {
Copy link
Owner

Choose a reason for hiding this comment

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

not required.

Copy link
Author

@zhiburt zhiburt May 13, 2022

Choose a reason for hiding this comment

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

Maybe not; but isn't it better with it?:)

I mean personally I like 2calls to SerializePyObject::new

rather then 2 constructors

SerializePyObject {
   py,
   include_none,
   obj: obj.extract(py)?,
}

let include_none = include_none.extract::<&PyBool>(py)?.is_true();
let obj = obj.extract(py)?;

Ok(Self { py, include_none, obj })
}
}

impl<'p> Serialize for SerializePyObject<'p> {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
Expand Down Expand Up @@ -83,23 +93,28 @@ impl<'p, 'a> Serialize for SerializePyObject<'p, 'a> {

macro_rules! add_to_map {
($map:ident, $key:ident, $value:ident) => {
if $key.is_none() {
$map.serialize_key("null")?;
} else if let Ok(key) = $key.extract::<bool>() {
$map.serialize_key(if key { "true" } else { "false" })?;
} else if let Ok(key) = $key.str() {
let key = key.to_string();
$map.serialize_key(&key)?;
} else {
return Err(ser::Error::custom(format_args!(
"Dictionary key is not a string: {:?}",
$key
)));
let ignore_none_entry = !self.include_none && ($value.is_none() || $key.is_none());
Copy link
Owner

Choose a reason for hiding this comment

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

I suspect we can do this in a more effecient way, e.g. only check instead the $key.is_none() logic etc.

Copy link
Author

Choose a reason for hiding this comment

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

Do you mean only check $key.is_none()?

Because it would break the tests.

if !ignore_none_entry {
if $key.is_none() {
$map.serialize_key("null")?;
} else if let Ok(key) = $key.extract::<bool>() {
$map.serialize_key(if key { "true" } else { "false" })?;
} else if let Ok(key) = $key.str() {
let key = key.to_string();
$map.serialize_key(&key)?;
} else {
return Err(ser::Error::custom(format_args!(
"Dictionary key is not a string: {:?}",
$key
)));
}

$map.serialize_value(&SerializePyObject {
py: self.py,
obj: $value,
include_none: self.include_none,
})?;
}
$map.serialize_value(&SerializePyObject {
py: self.py,
obj: $value,
})?;
};
}

Expand Down Expand Up @@ -135,9 +150,15 @@ impl<'p, 'a> Serialize for SerializePyObject<'p, 'a> {
cast!(|x: $type| {
let mut seq = serializer.serialize_seq(Some(x.len()))?;
for element in x {
if !self.include_none && element.is_none() {
// None values must be omitted
continue;
}

seq.serialize_element(&SerializePyObject {
py: self.py,
obj: element,
include_none: self.include_none,
})?
}
return seq.end();
Expand Down Expand Up @@ -183,24 +204,18 @@ impl<'p, 'a> Serialize for SerializePyObject<'p, 'a> {
}

#[pyfunction]
fn serialize(py: Python, obj: PyObject) -> PyResult<String> {
let s = SerializePyObject {
py,
obj: obj.extract(py)?,
};
match to_toml_string(&s) {
fn serialize(py: Python, obj: PyObject, include_none: PyObject) -> PyResult<String> {
zhiburt marked this conversation as resolved.
Show resolved Hide resolved
let serializer = SerializePyObject::new(py, &obj, &include_none)?;
match to_toml_string(&serializer) {
Ok(s) => Ok(s),
Err(e) => Err(TomlSerializationError::new_err(e.to_string())),
}
}

#[pyfunction]
fn serialize_pretty(py: Python, obj: PyObject) -> PyResult<String> {
let s = SerializePyObject {
py,
obj: obj.extract(py)?,
};
match to_toml_string_pretty(&s) {
fn serialize_pretty(py: Python, obj: PyObject, include_none: PyObject) -> PyResult<String> {
let serializer = SerializePyObject::new(py, &obj, &include_none)?;
match to_toml_string_pretty(&serializer) {
Ok(s) => Ok(s),
Err(e) => Err(TomlSerializationError::new_err(e.to_string())),
}
Expand Down
46 changes: 46 additions & 0 deletions tests/test_dump.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,49 @@ def test_dump_file(tmp_path):

def test_varied_list():
assert rtoml.dumps({'test': [1, '2']}) == 'test = [1, "2"]\n'


@pytest.mark.parametrize(
'input_obj,output_toml',
[
({None: 1}, ''),
({'key': None}, ''),
Copy link
Owner

Choose a reason for hiding this comment

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

what about None as the key: {None: 1}?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch.
I did fix it now it doesn't serialize anything.

({'foo': 'bar', 'key1': None}, 'foo = "bar"\n'),
({'key1': None, 'foo': 'bar'}, 'foo = "bar"\n'),
({'key1': None, 'foo': 'bar', 'key2': None}, 'foo = "bar"\n'),
],
)
def test_none_map_value(input_obj, output_toml):
assert rtoml.dumps(input_obj, include_none=False) == output_toml

@pytest.mark.parametrize(
'input_obj,output_toml',
[
([None], '[]'),
(["a", None], '["a"]'),
([None, "b"], '["b"]'),
(["a", None, "b"], '["a", "b"]'),
({'foo': 'bar', 'list': [None]}, 'foo = "bar"\nlist = []\n'),
({'foo': 'bar', 'list': [None, "b"]}, 'foo = "bar"\nlist = ["b"]\n'),
({'foo': 'bar', 'list': ["a", None]}, 'foo = "bar"\nlist = ["a"]\n'),
({'foo': 'bar', 'list': ["a", None, "b"]}, 'foo = "bar"\nlist = ["a", "b"]\n'),
],
)
def test_none_values_inside_list(input_obj, output_toml):
assert rtoml.dumps(input_obj, include_none=False) == output_toml

@pytest.mark.parametrize(
'input_obj,output_toml',
[
((None), '"null"'),
(("a", None), '["a"]'),
((None, "b"), '["b"]'),
(("a", None, "b"), '["a", "b"]'),
({'foo': 'bar', 'list': (None)}, 'foo = "bar"\n'),
({'foo': 'bar', 'list': (None, "b")}, 'foo = "bar"\nlist = ["b"]\n'),
({'foo': 'bar', 'list': ("a", None)}, 'foo = "bar"\nlist = ["a"]\n'),
({'foo': 'bar', 'list': ("a", None, "b")}, 'foo = "bar"\nlist = ["a", "b"]\n'),
],
)
def test_none_values_inside_tuple(input_obj, output_toml):
assert rtoml.dumps(input_obj, include_none=False) == output_toml
4 changes: 4 additions & 0 deletions tests/test_load.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,7 @@ def test_subtable():
smooth = true
"""
assert rtoml.loads(s) == ...


def test_empty_toml():
assert rtoml.loads('') == {}