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 pn.panel and ensure all Panes have layout attribute #126

Merged
merged 18 commits into from Oct 24, 2018
Merged

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Oct 24, 2018

@jbednar Please test this extensively.

  • Implements #124
  • Updates unit tests
  • Update docs

@codecov-io
Copy link

@codecov-io codecov-io commented Oct 24, 2018

Codecov Report

Merging #126 into master will decrease coverage by 0.24%.
The diff coverage is 94.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #126      +/-   ##
==========================================
- Coverage      90%   89.75%   -0.25%     
==========================================
  Files          27       27              
  Lines        4061     4023      -38     
==========================================
- Hits         3655     3611      -44     
- Misses        406      412       +6
Impacted Files Coverage Δ
panel/tests/test_vega.py 96.96% <ø> (-0.05%) ⬇️
panel/tests/test_interact.py 100% <100%> (ø) ⬆️
panel/tests/test_layout.py 100% <100%> (ø) ⬆️
panel/plotly.py 100% <100%> (ø) ⬆️
panel/tests/test_param.py 99.61% <100%> (-0.01%) ⬇️
panel/__init__.py 62.5% <100%> (ø) ⬆️
panel/interact.py 71.7% <75%> (-5.84%) ⬇️
panel/param.py 90.27% <81.81%> (+1.04%) ⬆️
panel/holoviews.py 82.5% <88.88%> (-0.73%) ⬇️
panel/pane.py 81.94% <93.33%> (-0.12%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b352396...3c9e061. Read the comment docs.

Copy link
Member

@jbednar jbednar left a comment

Looks good. I'll test it.

panel/layout.py Show resolved Hide resolved
panel/layout.py Outdated
@@ -313,3 +320,6 @@ def _get_model(self, doc, root=None, parent=None, comm=None):
model = self._bokeh_model(**self._init_properties())
self._link_params(model, ['width', 'height'], doc, root, comm)
return model


PaneBase._default_layout = Row
Copy link
Member

@jbednar jbednar Oct 24, 2018

Choose a reason for hiding this comment

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

Should this not be a public class Parameter? Seems like something one could legitimately want to customize.

panel/pane.py Show resolved Hide resolved
panel/layout.py Show resolved Hide resolved
panel/layout.py Outdated Show resolved Hide resolved
@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Oct 24, 2018

Always prefer if you warn me about making edits to a branch I'm actively working on. You can now also leave suggested edits directly in the GitHub, which let's me apply the changes when I'm ready.

@jbednar
Copy link
Member

@jbednar jbednar commented Oct 24, 2018

@jlstevens, consider code like this:

header = pn.Row(pn.create(oggm_logo, width=170), pn.Spacer(width=50), 
                pn.Column(pn.Pane(title, height=25, width=400), pn.Spacer(height=-15), pn.create(instruction, width=500)),
                pn.Spacer(width=180), pn.Column(pn.create(dyn_count), clear_button, pn.Spacer(height=-15)), 
                pn.create(pv_logo, width=80))

To me create reads oddly there; I'd rather see panel so that I can see that it's a row of rows, spacers, and panels, not a row of creates...

"cell_type": "markdown",
"metadata": {},
"source": [
"One thing to note is that since a ``Pane`` may define multiple views, when working with an explicitly constructed ``Pane`` care should be taken to compose the component of the ``Pane.layout`` manually."
Copy link
Member

@jbednar jbednar Oct 24, 2018

Choose a reason for hiding this comment

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

the components?

@philippjfr philippjfr changed the title Add panel.create and ensure all Panes have layout attribute Add pn.panel and ensure all Panes have layout attribute Oct 24, 2018
@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Oct 24, 2018

@jbednar Any objections to merging this soon?

@philippjfr philippjfr merged commit 2e36cfc into master Oct 24, 2018
2 checks passed
@philippjfr philippjfr deleted the create_layout branch Mar 6, 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.

None yet

3 participants