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

[WIP] Make ManifestSchema compatible with marshmallow >= 3 #3296

Closed

Conversation

@StefanBruens
Copy link

StefanBruens commented Dec 2, 2019

Instead of suppressing the exception, checking for an error and raising
a PlatformIOException, directly raise the PlatformIOException from
handle_error().

Add a small wrapper for Schema.load() to cover the the return value
difference ((data, error) tuple vs data only).

For more information, see
https://marshmallow.readthedocs.io/en/stable/upgrading.html

@StefanBruens StefanBruens force-pushed the StefanBruens:marshmallow3_support branch 2 times, most recently from 9dfc3fd to eb42f2c Dec 2, 2019
Instead of suppressing the exception, checking for an error and raising
a PlatformIOException, directly raise the PlatformIOException from
handle_error().

Add a small wrapper for Schema.load() to cover the the return value
difference ((data, error) tuple vs data only).

For more information, see
https://marshmallow.readthedocs.io/en/stable/upgrading.html
@StefanBruens StefanBruens force-pushed the StefanBruens:marshmallow3_support branch from eb42f2c to ec931bf Dec 2, 2019
@ivankravets

This comment has been minimized.

Copy link
Member

ivankravets commented Dec 2, 2019

Could you fix a PR to make it compatible between Python 2 and Python 3?

@StefanBruens StefanBruens changed the title Make ManifestSchema compatible with marshmallow >= 3 [WIP] Make ManifestSchema compatible with marshmallow >= 3 Dec 2, 2019
@StefanBruens

This comment has been minimized.

Copy link
Author

StefanBruens commented Dec 2, 2019

Supporting both Python2 and 3 is actually a little bit more work, but I am on it ...


from platformio.package.exception import ManifestValidationError
from platformio.util import memoized


class StrictSchema(Schema):
def handle_error(self, error, data):
def handle_error(self, error, data, *, _many, **_kwargs):

This comment has been minimized.

Copy link
@hmaarrfk

hmaarrfk Dec 14, 2019

Python 2.7 doesn't have keyword only arguments

https://www.python.org/dev/peps/pep-3102/

Suggested change
def handle_error(self, error, data, *, _many, **_kwargs):
def handle_error(self, error, data, _many, **_kwargs):
data, _ = self.load(manifest)
return data

def handle_error(self, error, data, *, _many, **_kwargs):

This comment has been minimized.

Copy link
@hmaarrfk

hmaarrfk Dec 14, 2019

Suggested change
def handle_error(self, error, data, *, _many, **_kwargs):
def handle_error(self, error, data, _many, **_kwargs):

No keyword only arguments in Python 2.7

@hmaarrfk

This comment has been minimized.

Copy link

hmaarrfk commented Dec 14, 2019

Sorry for jumping in, just did a quick browse and noticed at least one feature that isn't available in Python 2.7.

@ivankravets ivankravets added this to the 4.1.1 milestone Dec 17, 2019
hmaarrfk added a commit to hmaarrfk/platformio-core that referenced this pull request Dec 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.