-
Notifications
You must be signed in to change notification settings - Fork 317
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
Revert changes made in pr #537 to change DataTransfer.data typehint. #570
Revert changes made in pr #537 to change DataTransfer.data typehint. #570
Conversation
…ta typehint. The typehint should be a str type, rather than Any.
@mdwcrft Does this PR have any impact on our internal codebase? |
As discussed offline I think this should still be of type |
The schema specifies a string here. |
I would suggest also that we add a comment to explain this complication in the standard in detail - I might prevent it reverted again into the future? |
31a335a
to
a71fc59
Compare
@Jared-Newell-Mobility ready for review (once tests pass) |
I've check over things again, I don't see a type specified in the schema. There is clearly a discrepancy between the descriptions 1. Optional. Data without specified length or format, in response to request and 2. text, data without specified length or format. Schema normally trumps; I'm going to raise it with the OCA at minimum this might be cleared up also for others in the next Errata
|
The OCA has confirmed that AnyType is any type as expected and is reflected in the schema. However, in OCPP 2.0.1 Part 2, Section 2.1.4. Primitive Datatypes the description for AnyType could be improved on, that is to remove the word "Text". This has been raised. So given this, I will close this PR as the current type used is correct. |
Thanks for your effort @Jared-Newell-Mobility |
According to this issue, a PR was made to change the type of
DataTransfer.data
toAny
rather thanstr
.This PR makes the type
str
instead ofAny
.