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

Add deprecation warning if user supplies iterables and user-defined s… #5187

Merged
merged 2 commits into from Sep 20, 2016

Conversation

@Maggie-M
Copy link
Contributor

@Maggie-M Maggie-M commented Sep 19, 2016

issues: fixes #2056

…ource to glyph methods
@bryevdv
Copy link
Member

@bryevdv bryevdv commented Sep 19, 2016

@bokeh/dev I propose to deprecate this now in 0.12.3, and raise a RuntimeError in the very near future (and add a test for the exception at that time)

@bryevdv bryevdv added the MIGRATION label Sep 19, 2016
@bryevdv
Copy link
Member

@bryevdv bryevdv commented Sep 19, 2016

@bokeh/dev any comments on this before merging?

@mattpap
Copy link
Contributor

@mattpap mattpap commented Sep 19, 2016

@bryevdv, if this is a deprecation, then it should use appropriate machinery we have in bokeh for this.

@ghost
Copy link

@ghost ghost commented Sep 19, 2016

@mattpap we have deprecation machinery for functions and modules, what machinery would cover this case?

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Sep 19, 2016

@mattpap do you mean this?

warn(message, BokehDeprecationWarning, stacklevel=2)

? If so, sure that seems reasonable

@mattpap
Copy link
Contributor

@mattpap mattpap commented Sep 19, 2016

@bryevdv, at minimum this, because BokehDeprecationWarning gets special treatment to be always visible. However, I thought we had a function for this bokeh.util.deprecate.

@ghost
Copy link

@ghost ghost commented Sep 19, 2016

@mattpap That's just a function decorator

@Maggie-M can you change the warning as shown above?

@bryevdv bryevdv merged commit 9c73877 into master Sep 20, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bryevdv bryevdv deleted the maggie-m/2056_glyph_sources branch Sep 20, 2016
@mattpap
Copy link
Contributor

@mattpap mattpap commented Sep 21, 2016

@bryevdv, unfortunately examples weren't updated, so there is some annoying text output (e.g. plotting/server/hover).

@ghost
Copy link

@ghost ghost commented Sep 21, 2016

Ah, I think that was the only one, thanks for the catch

@ghost
Copy link

@ghost ghost commented Sep 21, 2016

Just going to have @Maggie-M delete it as part of #5154 since it is the only output_server example as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.