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

Improve docs example in first_steps_8.rst #13161

Merged
merged 2 commits into from
Jul 13, 2023

Conversation

cdeil
Copy link
Contributor

@cdeil cdeil commented May 27, 2023

This is a tiny improvement to a docs example, adding a show() call to have a full working example and be consistent with the other examples in the getting started guide.

Filing a [BUG] for such small docs improvements that aren't really bugs seems overkill. Hope it's OK as an exception.

@codecov
Copy link

codecov bot commented May 27, 2023

Codecov Report

Merging #13161 (4546f65) into branch-3.2 (bb3a605) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##           branch-3.2   #13161      +/-   ##
==============================================
- Coverage       92.42%   92.41%   -0.01%     
==============================================
  Files             316      315       -1     
  Lines           20040    20038       -2     
==============================================
- Hits            18521    18518       -3     
- Misses           1519     1520       +1     

@bryevdv
Copy link
Member

bryevdv commented May 27, 2023

cc @tcmetzger IIRC omitting the show here was an intentional choice.

@tcmetzger
Copy link
Member

I honestly don't remember why there is no show() in this example, I don't really see a reason why it shouldn't be added. This example looks quite boring if rendered, and the point of using a different data structure as input isn't really illustrated by displaying the plot, but none of that should stop us from having show() in there for consistency.

If we do add show(), how about we make the figure a little smaller (by defining height=250) and make the circles a little bigger (size=20), for example like this:

from bokeh.plotting import figure, show
from bokeh.models import ColumnDataSource
# create dict as basis for ColumnDataSource
data = {'x_values': [1, 2, 3, 4, 5],
        'y_values': [6, 7, 2, 3, 6]}
# create ColumnDataSource based on dict
source = ColumnDataSource(data=data)
# create a plot and renderer with ColumnDataSource data
p = figure(height=250)
p.circle(x='x_values', y='y_values', size=20, source=source)
show(p)

@cdeil
Copy link
Contributor Author

cdeil commented May 28, 2023

@tcmetzger - I applied your suggestion in 4546f65

@cdeil
Copy link
Contributor Author

cdeil commented May 28, 2023

the point of using a different data structure as input isn't really illustrated by displaying the plot

True. The key question isn't explained: "Why does ColumnDataSource and the other classes exist? Even if they exist internally, why should I as user care or use them?"

none of that should stop us from having show() in there for consistency

I think simply having full working examples that newbies can copy & paste is the biggest advantage. This actually happened to me: I copy & pasted the example and didn't get a plot. Took me a minute to figure out that I was simply missing the "show" call, but the example otherwise was working perfectly.


I do think it would be good to have some information on "why" ColumnDataSource and the filters exist, and "when" I have to or should use them over simple pandas DataFrames, and also what the performance implications are when bokeh makes copies. But that's a different possible aspect of the docs that could be improved. At least I don't know the answer and can't add it here (if you agree something could be added).

@bryevdv
Copy link
Member

bryevdv commented Jul 13, 2023

Thanks for the PR @cdeil !

@bryevdv bryevdv changed the base branch from branch-3.2 to branch-3.3 July 13, 2023 15:39
@bryevdv bryevdv added this to the 3.3 milestone Jul 13, 2023
@bryevdv bryevdv merged commit 969079b into bokeh:branch-3.3 Jul 13, 2023
27 checks passed
mattpap pushed a commit that referenced this pull request Jul 18, 2023
* Improve docs example in first_steps_8.rst

* Update figure in docs example first_steps_8.rst
@mattpap mattpap mentioned this pull request Jul 18, 2023
14 tasks
mattpap added a commit that referenced this pull request Jul 20, 2023
* Improve performance of WebGL line glyph (#13236)

* Improve performance of webgl line glyph

* Update baseline images

* Update ruff repo links (#13242)

* Fix `PropertyValueColumnData._stream()` to handle `rollover=0` (#13239)

* Fix `PropertyValueColumnData._stream()` to handle `rollover=0`

* Check 'event.data' in the unit tests

* Improve docs example in first_steps_8.rst (#13161)

* Improve docs example in first_steps_8.rst

* Update figure in docs example first_steps_8.rst

* Resolve issues with high DPI GridPlot exports (#13253)

* Allow to disable DatePicker (etc.) after creation (#13256)

* Use ../core/kinds instead of core/kinds in imports (#13254)

* Don't paint undisplayed plots (#13250)

* Update theme.py (#13270)

* Fixed Broken Link (#13266)

* Fixed Broken Link

* Update docs/bokeh/source/docs/first_steps/first_steps_9.rst

Co-authored-by: Bryan Van de Ven <bryan@bokeh.org>

---------

Co-authored-by: Bryan Van de Ven <bryan@bokeh.org>

* fix unterminated string literals in example code [skip ci] (#13274)

* Fix Legend's grid layout for uneven number of items (#13263)

* 13272 update legend docs (#13273)

* update docs for two dimensional legends

* Apply suggestions from code review

* Implement cloneable interface in ndarrays (#13232)

* Update docs/bokeh/switcher.json

* Added release notes

---------

Co-authored-by: Ian Thomas <ianthomas23@gmail.com>
Co-authored-by: Xiaoyang Liu <siujoeng.lau@gmail.com>
Co-authored-by: Christoph Deil <Deil.Christoph@gmail.com>
Co-authored-by: Moritz Schreiber <68053396+mosc9575@users.noreply.github.com>
Co-authored-by: Rajat Shenoi <rajatshenoi@outlook.com>
Co-authored-by: Bryan Van de Ven <bryan@bokeh.org>
@bryevdv bryevdv modified the milestones: 3.3, 3.2.1 Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants