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

Make yaml features optional #251

Merged
merged 5 commits into from Jul 29, 2018

Conversation

@lafrech
Copy link
Member

commented Jul 20, 2018

Addresses part of #246.

TODO:

  • Update docs
  • Update changelog

Points to check:

setup.py: I added 'yaml' in extra_requires

In APISpec class, yaml stuff is imported when calling to_yaml and ImportError is not caught:

    def to_yaml(self):
        from .yaml_utils import dict_to_yaml
        return dict_to_yaml(self.to_dict())

@lafrech lafrech force-pushed the Nobatek:dev_yaml_optional branch from 7afd06c to 75ccb8c Jul 20, 2018

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Jul 20, 2018

I forgot to mention that this PR is based on #249.

Also, regarding APISpec.to_yaml, there are alternatives to importing yaml_utils in the method:

  • Remove the method and let the user do that manually (import dict_to_yaml and call to_dict).
  • Propose an APISpecYAML mixin in yaml_utils.

But adding a warning in APISpec.to_yaml docstring should be enough.

@sloria

sloria approved these changes Jul 29, 2018

Copy link
Member

left a comment

LGTM. @lafrech Can you update this with dev? Once that's done, go ahead an merge this.

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Jul 29, 2018

I think it is already up-to-date with dev. It includes the commits in #249.

The docs might need an update, though. Can't promise when I'll do it (summer holidays).

sloria added some commits Jul 29, 2018

@sloria

This comment has been minimized.

Copy link
Member

commented Jul 29, 2018

No prob; I've gone ahead and updated the docs and changelog.

- [apispec.core]: *Backwards-incompatible*: YAML support is optional. To
install with YAML support, use ``pip install 'apispec[yaml]'``. You
will need to do this if you use the ``apispec.ext.flask`` or
``apispec.ext.tornado`` plugins.

This comment has been minimized.

Copy link
@lafrech

lafrech Jul 29, 2018

Author Member

FlaskPlugin, TornadoPlugin, or BottlePlugin

This comment has been minimized.

Copy link
@sloria

sloria Jul 29, 2018

Member

Good catch; fixed.

@sloria sloria merged commit 67a4e76 into marshmallow-code:dev Jul 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lafrech lafrech deleted the Nobatek:dev_yaml_optional branch Jul 29, 2018

@lafrech lafrech referenced this pull request Feb 11, 2019

lafrech added a commit that referenced this pull request Feb 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.