Added support for infinite and nan floats/doubles #215
Conversation
|
|
||
| Parameters | ||
| ---------- | ||
| value : Any |
There was a problem hiding this comment.
I wouldn't document this. I also think value shouldn't be Any and rather str
There was a problem hiding this comment.
I think it can be a float too; will check and update.
|
|
||
| Parameters | ||
| ---------- | ||
| value : float |
There was a problem hiding this comment.
I again wouldn't document this
tests/test_inputs.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| l : List |
There was a problem hiding this comment.
| l : List | |
| l: List |
There was a problem hiding this comment.
This is in the NumPy docstring spec: https://numpydoc.readthedocs.io/en/latest/format.html#sections
There was a problem hiding this comment.
It's not how the other functions are documented
tests/test_inputs.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| l : Dict[Any, Any] |
There was a problem hiding this comment.
| l : Dict[Any, Any] | |
| l: Dict[Any, Any] |
What’s the downside? Documenting anything makes it easier for new people to contribute to the package; removing committed comments unnecessarily increases the barrier to entry. |
|
It's more to maintain |
| message Test { | ||
| double positive = 1; | ||
| double negative = 2; | ||
| double nan = 3; |
There was a problem hiding this comment.
this makes me realise that I don't see any explicit test of regular floats anywhere!
Could we generalise this test case a little for completeness?
| double nan = 3; | |
| double nan = 3; | |
| double zero = 4 | |
| double three_point_one_four = 5 | |
| double neg_three_point_one_four = 6 |
- Added support for the custom double values from the protobuf json spec: "Infinity", "-Infinity", and "NaN" - Added `infinite_floats` test data - Updated Message.__eq__ to consider nan values equal - Updated `test_message_json` and `test_binary_compatibility` to replace NaN float values in dictionaries before comparison (because two NaN values are not equal)
|
Rebased to resolve conflicts and enhanced the tests. |
Resolves #214.
This pull request adds support for the custom double values defined in the protobuf spec:
"Infinity""-Infinity""Nan"Support is added by updates to the
Message.to_dictandMessage.from_dictmethods, and not viajson.loads, to ensure that onlydoubleandfloatfields will have their values casted tofloat("inf"), etc.Furthermore,
Message.__eq__has been updated to consider NaN values equal, otherwise a message may not be equal to itselftest_message_jsonandtest_binary_compatibilitytest functions have been updated to use the newdict_replace_nansfunction that replaces NaN float values with the string"NaN"(becauseassert float("nan") == float("nan")always fails).infinite_floatstest to check correctness; this works with theprotobufpackage and fails in the currentmasterbranchWarning
For the sake of clarity,
json.loads(message.to_json())is not equal to itself in the case that a message contains NaN values.