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.

Sign up for GitHub

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

Jump to bottom

Fixed output magic message when backend unavailable #1045

Merged
merged 4 commits into from
Jan 10, 2017
Merged

Fixed output magic message when backend unavailable #1045

merged 4 commits into from
Jan 10, 2017

Conversation

jlstevens
Copy link
Contributor

@jlstevens jlstevens commented Jan 10, 2017 •
edited
Loading

This PR addresses #999 regarding the inappropriate error message when trying to load an unavailable backend. Here is the old behavior:

image

And the new behavior:

image

I've tried to make a minimal fix. That said, the output message might suggest how to load the backend with notebook_extension...

@jlstevens
Copy link
Contributor Author

Now with a custom exception message:

image

I think this PR is ready to review/merge. I am happy to improve the exception message if you have other suggestions.

@jlstevens jlstevens requested a review from philippjfr January 10, 2017 17:39
@jbednar
Copy link
Member

jbednar commented Jan 10, 2017 •
edited
Loading

What will happen if you ask for backend='powerpoint', i.e. one that truly doesn't exist? it would be nice if it were clear whether it's just nonsense/typo, or if there was a missing step. I understand that might not be practical, of course.

@jlstevens
Copy link
Contributor Author

Good suggestion! I don't think it would be too hard though I don't know if we have a list of all possible backend names somewhere I can use...

jbednar
jbednar reviewed Jan 10, 2017
View reviewed changes
holoviews/ipython/magics.py


raise ValueError(OutputMagic.backend_list, value)
# raise ValueError("Backend %r not available. Has it been loaded with the notebook_extension?" % value)
Copy link
Member

Choose a reason for hiding this comment

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

Stray comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@jlstevens
Copy link
Contributor Author

With the last commit, you have two cases:

image

and

image

I think this PR is now ready for final review/merge.

@jbednar
Copy link
Member

jbednar commented Jan 10, 2017

Looks good.

@jbednar jbednar merged commit 642a604 into master Jan 10, 2017
@jbednar jbednar deleted the backend_index_range_fix branch January 10, 2017 20:33
@github-actions GitHub Actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jbednar jbednar jbednar left review comments

@philippjfr philippjfr Awaiting requested review from philippjfr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jlstevens @jbednar