Skip to content

Provide option to use data keys as property name and title#65

Open
smith-m wants to merge 6 commits intofuhrysteve:masterfrom
smith-m:feature/useDataKeys
Open

Provide option to use data keys as property name and title#65
smith-m wants to merge 6 commits intofuhrysteve:masterfrom
smith-m:feature/useDataKeys

Conversation

@smith-m
Copy link

@smith-m smith-m commented Aug 23, 2018

provide option to use data_keys or load_from and dump_to as title

Use as JSONSchema(prefer_data_key=True) to enable

@smith-m
Copy link
Author

smith-m commented Aug 23, 2018

Relates to #64

@coveralls
Copy link

coveralls commented Aug 23, 2018

Coverage Status

Coverage decreased (-0.6%) to 98.675% when pulling bf853fa on smith-m:feature/useDataKeys into 3e0891a on fuhrysteve:master.

@smith-m
Copy link
Author

smith-m commented Aug 23, 2018

Code coverage reduced because data_keys vs load_from / dump_to behavior differs between marshmallow 2 v 3. One statement has been added to differentiate between marshmallow version for this purpose. Current testing does not seem to work for marshmallow v3. Is there a recommended way to test a library across multiple versions of a dependency?

@smith-m
Copy link
Author

smith-m commented Aug 23, 2018

Also, I will add example of this functionality to readme if this will be merged

setup.py Outdated
include_package_data=True,
install_requires=['marshmallow>=2.9.0'],
tests_require=['pytest>=2.9.2', 'jsonschema', 'strict-rfc3339', 'coverage>=4.1'],
tests_require=['pytest>=2.9.2', 'jsonschema<=2.5.1', 'strict-rfc3339', 'coverage>=4.1'],
Copy link
Owner

Choose a reason for hiding this comment

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

Oh boy - what's all this now?

Copy link
Author

Choose a reason for hiding this comment

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

something like 11 tests failed with jsonschema==2.6.0 - I can check again and validate.

Copy link
Author

Choose a reason for hiding this comment

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

I've removed this - tests work without the change.

@fuhrysteve
Copy link
Owner

Looking good! I'll spend some time looking it over later this week!

@fuhrysteve
Copy link
Owner

@smith-m If you can look over and merge this in:

smith-m#1

That should fix the tests I think! It does for me locally, anyways

fix build and make test suite compatible with marshmallow 2 and 3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants