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

Adding ICO and pathlib support for image #2757

Merged
merged 5 commits into from
Sep 21, 2021
Merged

Adding ICO and pathlib support for image #2757

merged 5 commits into from
Sep 21, 2021

Conversation

hoxbro
Copy link
Member

@hoxbro hoxbro commented Sep 19, 2021

Added a ICO class to pane.image and pathlib conversion to pane.image and favicon in templates.

@philippjfr
Copy link
Member

Not sure it's worth it to make a reference gallery item for ICO but we need to exclude it from the unit test that checks such an example exists.

@codecov
Copy link

codecov bot commented Sep 20, 2021

Codecov Report

Merging #2757 (2e6f3d1) into master (80a66c3) will increase coverage by 0.63%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2757      +/-   ##
==========================================
+ Coverage   82.03%   82.67%   +0.63%     
==========================================
  Files         187      188       +1     
  Lines       23963    24070     +107     
==========================================
+ Hits        19658    19899     +241     
+ Misses       4305     4171     -134     
Impacted Files Coverage Δ
panel/template/base.py 78.71% <40.00%> (-0.08%) ⬇️
panel/pane/__init__.py 100.00% <100.00%> (ø)
panel/pane/image.py 80.72% <100.00%> (+2.39%) ⬆️
panel/tests/pane/test_image.py 100.00% <100.00%> (ø)
panel/tests/test_docs.py 100.00% <100.00%> (ø)
panel/tests/command/test_serve.py 85.00% <0.00%> (ø)
panel/tests/conftest.py 95.90% <0.00%> (+0.21%) ⬆️
panel/pane/holoviews.py 79.65% <0.00%> (+0.21%) ⬆️
panel/pane/vtk/vtk.py 96.83% <0.00%> (+0.45%) ⬆️
panel/pane/plot.py 59.69% <0.00%> (+0.51%) ⬆️
... and 12 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 80a66c3...2e6f3d1. Read the comment docs.

@hoxbro
Copy link
Member Author

hoxbro commented Sep 20, 2021

I don't either see the need for a reference gallery item.

Should I also make the logo in BasicTemplate accept pathlib?

@philippjfr
Copy link
Member

Should I also make the logo in BasicTemplate accept pathlib?

Yes, but maybe in a separate PR.

@philippjfr philippjfr merged commit 6a9e254 into holoviz:master Sep 21, 2021
philippjfr pushed a commit that referenced this pull request Sep 24, 2021
* Added icon pane

* Added f to string

* Added pathlib for images and template favicon

* Added tests

* Removed ICO from test_panes_are_in_reference_gallery
@MarcSkovMadsen
Copy link
Collaborator

Without a reference example, how would users know its supported? And what potential arguments are available?

@hoxbro
Copy link
Member Author

hoxbro commented Apr 13, 2022

I'm not opposed to adding a reference gallery of ICO.

Though, I don't think anybody will ever use an ICO pane directly but will rather pass a path or URL of an .ico file into a theme which will then just work. E.g., like what you did with awesome-panel.

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.

3 participants