Handle recursively serializing a dataclasses as a dictionary. #547
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This package models calls, call_results and types using `dataclass`es. Optional fields are marked as so using the `typing.Optional` hint. E.g.: ``` from typing import Optional @DataClass class EVSEType: id: int connector_id: Optional[int] = None @DataClass class ChangeAvailabilityRequestPayload: operational_status: str evse: Optional[EVSEType] = None ``` The OCPP specification requires to remove empty optional fields from the JSON. Currently, that is done using `ocpp.charge_point.remove_nones()`. However, this function is pretty bad. It removes all top level fields which value is `None` only. Nested structures are not supported. Also, `remove_nones()` doesn't verify if the field is optional. The following snipped highlights `remove_nones()` shortcomings. The field `evse.connector_id` is not removed, although it's value is `None`. ``` python from dataclasses import asdict from ocpp.charge_point import remove_nones request = ChangeAvailabilityRequestPayload( operational_status="Operative", evse=EVSEType( id=1 ) ) assert remove_nones(asdict(request)) == { 'operational_status': 'Operative', 'evse': { 'id': 1, 'connector_id': None, } } ``` @MacDue proposed a better solution[0]. This commit implements that solution. We can't merge this yet, however. First, it's backwards incompatible. We need to figure out if we want to stay backwards compatible. And if so, we need to figure out how we're going to do that. Second, we need to provide type hints correctly across the package. A lot of calls and call_results are missing correct type hints for nested structures. Take for example `ocpp.v201.call.ChangeAvailabilityRequestPayload`. It's not defined as above, instead it looks like[1]: ``` @DataClass class ChangeAvailabilityPayload: operational_status: str evse: Optional[Dict] = None ``` This commit depends on correct and exact type hints. So `Optional[EVSEType]` instead of `Optional[Dict]`. Relates to #255 [0]: #255 (comment) [1]: https://github.com/mobilityhouse/ocpp/blob/efc8f081d063bb43680fd8f2b8c861a71356d784/ocpp/v201/call.py#L29-L32
# Conflicts: # ocpp/charge_point.py # poetry.lock # pyproject.toml # tests/test_charge_point.py
OrangeTux
requested changes
Jan 10, 2024
pyproject.toml
Outdated
| @@ -33,6 +33,7 @@ classifiers = [ | |||
| [tool.poetry.dependencies] | |||
| python = "^3.7" | |||
| jsonschema = "^4.4.0" | |||
| typing-extensions = "^4.0.0" | |||
There was a problem hiding this comment.
Since we drop support for Python 3.7, this dependency is not required right?
typing.get_args and typing.get_origin are both available in Python 3.8,
There was a problem hiding this comment.
Yeah - Corrected with commit f59f346
ocpp/charge_point.py
Outdated
Comment on lines
103
to
104
| with_additional_info = StatusInfoType(reason="Unknown", | ||
| additional_info="More details") |
There was a problem hiding this comment.
Can you put this on a single line, to make the code sample valid Python?
There was a problem hiding this comment.
Done - satisfied with b3c62d3
ocpp/charge_point.py
Outdated
| return get_origin(field.type) is Union and type(None) in get_args(field.type) | ||
|
|
||
|
|
||
| def serialize_as_dict(dataclass, remove_empty_optional_fields: bool = True): |
There was a problem hiding this comment.
I think the solution can be simplified.
The root of issue #255 is that dataclasses.asdict() only serializes the top level dataclass as a dictionary. Any inner dataclasses are not serializes as a dict.
The body of this function fixes that: recursively serializing a dataclasses as a dictionary.
But this function does more:
- remove any optional fields containing
None. - change casing to snake_case.
I think we can remove the latter to features of this function.
And, in lines 318 and 378, replace the call to dataclass.asdict() with serialize_as_dict().
There was a problem hiding this comment.
asdict() corrected in commit d160959
There was a problem hiding this comment.
Removed the more with commit 2834dbe
# Conflicts: # poetry.lock # tests/test_charge_point.py
Jared-Newell-Mobility
changed the title
None
OrangeTux
previously approved these changes
Feb 13, 2024
Jared-Newell-Mobility
deleted the
improve-remove-optional-empty-fields
branch
Merged
Jared-Newell-Mobility
added a commit
that referenced
this pull request
Feb 14, 2024
## 1.0.0-rc.1 (2024-02-14) - [#573](#573) Introduce Experimental Module For v2.1 - [#547](#547) Feat: Handle recursively serializing a dataclasses as a dictionary Thanks [@MacDue](https://github.com/MacDue) - [#601](#601) Fix case conversion for soc in non "State of Charge" context - [#523](#523) The serialisation of soc to SoC should not occur in camel case if it is existing at the beginning of a field - [#515](#515) Update Readthedocs configuration - [#602](#602) Correct v2g serialisation/deserialisation - [#557](#557) OCPP 2.0.1 Wrong data type in CostUpdated total_cost - [#564](#564) Add support For Python 3.11 and 3.12 - [#583](#583) OCPP v1.6/v2.0.1 deprecate dataclasses from calls and call results with the suffix 'Payload' - [#590](#336) snake_to_camel_case url to URL does not get converted correctly - [#591](#591) Camel_to_snake_case doesn't handle v2x correctly - [#593](#593) Update tests to use Call and CallResult without the suffix Payload - [#435](#435) Typo in CostUpdated Action - [#577](#577) v2.0.1 AttributeType Enum Corrections - [#340](#340) 2.0.1 dataclasses have a incorrect types that don't match carnality - [#519](#519) Typo in v201.enums.StatusInfoReasonType.invaild_schedule - [#510](#510) v2.0.1 UnitOfMeasureType - Enums missing and update docstring to allow use for variableCharacteristics - [#508](#508) Exception - OccurrenceConstraintViolationError doc string correction ## DEPRECATED ## - [#599](#599) v1.6 Action Enum members corrected IMPORTANT SEE UPGRADE PATH [#599](#599) - [#579](#579) v2.0.1 Action enums corrected - IMPORTANT SEE UPGRADE PATH [#579](#579) ## BREAKING ## - [#574](#574) Remove v1.6 deprecated enum members - IMPORTANT see upgrade path [#574](#574) - [#498](#498) Remove support for OCPP 2.0 - IMPORTANT SEE UPGRADE PATH [#498](#498)
Jared-Newell-Mobility
added a commit
that referenced
this pull request
Apr 25, 2024
- OCPP v1.6/v2.0.1 deprecate dataclasses from calls and call results with the suffix 'Payload' by @Jared-Newell-Mobility in #584 - In GA, validate project against Python 3.11 and 3.12. by @Jared-Newell-Mobility in #589 - drop support for python 3.7 by @Jared-Newell-Mobility in #585 - Update Code Owners by @Jared-Newell-Mobility in #588 - Revert "drop support for python 3.7" by @Jared-Newell-Mobility in #597 - OCPP 2.0.1 Wrong data type in CostUpdated total_cost by @Jared-Newell-Mobility in #596 - Update tests to use Call and CallResult without the suffix Payload by @Jared-Newell-Mobility in #595 - Fix camel_to_snake_case for "v2x" by @Jared-Newell-Mobility in #594 - Correct naming of members of v201.enums.AttributeType by @Jared-Newell-Mobility in #578 - Drop support for python 3.7 by @Jared-Newell-Mobility in #598 - Remove v1.6 deprecated enum variants by @Jared-Newell-Mobility in #575 - Typo in CostUpdated Action #435 by @Jared-Newell-Mobility in #491- - Remove support for ocpp 2.0 by @Jared-Newell-Mobility in #576 - v201.datatypes.ChargingNeedsType.request_energy_transfer is mistyped by @Jared-Newell-Mobility in #496 - v201.enums.StatusInfoReasonType.invaild_schedule by @Jared-Newell-Mobility in #521 - update to match Appendix 2. Standardized Units Of Measure by @Jared-Newell-Mobility in #512 - v16/schemas/StopTransaction.json missing "Hertz" #207 by @Jared-Newell-Mobility in #497 - Correct v2g serialisation/deserialisation by @Jared-Newell-Mobility in #606 - 2.0.1 dataclasses have a incorrect types that don't match carnality by @Jared-Newell-Mobility in #529 - Readthedocs_configuration_is_outdated by @Jared-Newell-Mobility in #608 - Readthedocs_configuration_is_outdated_config_update by @Jared-Newell-Mobility in #609 - The serialisation of soc to SoC should not occur in camel case if it is existing at the beginning of a field by @Jared-Newell-Mobility in #527 - Fix case conversion for soc in non "State of Charge" context by @Jared-Newell-Mobility in #607 - Handle recursively serializing a dataclasses as a dictionary. by @Jared-Newell-Mobility in #547 - v2.0.1 Action enums corrected by @Jared-Newell-Mobility in #580 - URL does not get converted from snake_case responder_url to camelCase responderURL by @Jared-Newell-Mobility in #592 - v1.6 Action Enum members corrected by @Jared-Newell-Mobility in #600 - Introduce Experimental Module For v2.1 by @Jared-Newell-Mobility in #605 - Bump to 1.0.0-rc.1 by @Jared-Newell-Mobility in #611 - fix typo in DataTypeEnum -> value_too_high by @d2avids in #612 - fix typo CostUpdated enum for 201 by @OSkrk in #620
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@OrangeTux this may have been missed so I created a PR - see #255
Dependent on #475 - which removes support for python 3.7