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

Starting specs 1 and 2 #1

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Starting specs 1 and 2 #1

wants to merge 8 commits into from

Conversation

jsignell
Copy link
Member

I'm opening this pull request as a way to further the discussion in pyviz/pyviz.org#32 and hopefully arrive at something that we can all agree to.

I split the spec up by which section of the API is being targeted (top-level, figure objects, children of figures). I can make them separate PRs if that helps with discussion.

@jsignell
Copy link
Member Author

jsignell commented Nov 7, 2019

@story645 @munkm @jonmmease @tacaswell this came up again at PyData NYC, so I thought it wouldn't hurt to poke everybody :)

spec_01.md Outdated
Calling `.__spec_version__()` on the library should return a list of the specs that the library complies with. It should include the version of the spec that is being used.

```python
>>> LIBRARY.__spec_version__()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this be made more explicit? Something like __pyviz_version__ or __pyviz_plot_version__ (oh that gets long fast)? Main concerns are spec being too generic and future specifications by this group (plotting versus maps versus jupyter widget interface versus something else). Maybe they are all in one large specification, but want to point it out at least.

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe __pyviz_spec_version__?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. Good point, @djhoese .

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe just __pyviz_spec__.

spec_02.md Outdated
1) The top-level figure class has a `.show()` method that can be called without arguments to display the figure as a side-effect.
2) The optional `renderer` kwarg can be used to override the current default renderer. e.g.:
- `FIGURE.show(renderer='png')` to display the figure as a static png image/
- `FIGURE.show(renderer='browser')` to display the figure in a browser tab. This works in non-jupyter/IPython contexts.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What level of interactivity should be required from this show interface? And should there be a defined list of "preferred" rendering options that a library should support with defined names (ex. so one library doesn't use jpg while another uses jpeg).

Does browser mean interactive javascript based display? Or could it mean open a browser tab and display the PNG version of this figure? Does the browser rendering open a new tab if in a jupyter notebook environment or does it display it inline in the notebook cell's output?

Copy link
Member

Choose a reason for hiding this comment

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

I would argue for a minimal standard for show that can be met by all libraries, and thus would not assume any interactivity or JavaScript.

Copy link
Member Author

Choose a reason for hiding this comment

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

should this maybe be output instead of renderer so that it can match .save()?

Copy link
Member Author

Choose a reason for hiding this comment

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

should the output option just be 'html' then and a browser tab is implicitly where html would be displayed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that makes sense.

@jbednar
Copy link
Member

jbednar commented Nov 8, 2019

Can we make a sketch of a test suite that can determine whether a given library fulfills the spec?

Copy link
Member

@jbednar jbednar left a comment

Choose a reason for hiding this comment

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

Looks good for a first pass, to me. I've made some suggestions.

spec_01.md Outdated Show resolved Hide resolved
spec_01.md Outdated Show resolved Hide resolved
spec_01.md Show resolved Hide resolved
spec_01.md Outdated Show resolved Hide resolved
spec_01.md Outdated Show resolved Hide resolved
spec_02.md Outdated Show resolved Hide resolved
spec_02.md Outdated Show resolved Hide resolved
spec_03.md Outdated Show resolved Hide resolved
spec_03.md Outdated Show resolved Hide resolved
spec_03.md Outdated Show resolved Hide resolved
Co-Authored-By: James A. Bednar <[email protected]>
@jsignell
Copy link
Member Author

jsignell commented Nov 8, 2019

Can we make a sketch of a test suite that can determine whether a given library fulfills the spec?

I think that sounds like a good idea. Do you think it belongs in this repo?

@jbednar
Copy link
Member

jbednar commented Nov 8, 2019

Yes!

spec_01.md Outdated Show resolved Hide resolved
@jsignell jsignell changed the title Starting specs 1, 2, and 3 Starting specs 1 and 2 Dec 2, 2019
spec_02.md Outdated
If a given operation doesn't make sense for that library, then it can satisfy the spec by simply having that method return a message to that effect ("Unsupported: LIBRARY does not provide JSON output").

## Jupyter methods
Every library that supports rendering in Jupyter should support at least one of the various IPython rich display methods, i.e. `_repr_html_`, `_repr_png_`, `_ipython_display_`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe _repr_mimebundle_ along with a separately-installed frontend extension is the recommended way to do this according to the Jupyter team.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you propose just adding _repr_mimebundle_ to the list, or enforcing that it should be implemented?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think adding it to the list is fine, but folks from the Jupyter team may have stronger opinions.

@jbednar jbednar mentioned this pull request Dec 3, 2019
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.

4 participants