Skip to content

Calendar visualizations using bokeh #107

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

Merged
merged 8 commits into from
Sep 29, 2022
Merged

Calendar visualizations using bokeh #107

merged 8 commits into from
Sep 29, 2022

Conversation

BSchilperoort
Copy link
Contributor

@BSchilperoort BSchilperoort commented Sep 26, 2022

This PR adds "bokeh" as an alternative backend for the visualization utility.

The advantage of bokeh over matplotlib is that it allows for interactive plots in jupyter notebooks. Tooltips can then provide more detail on each interval.

This PR adds a basic implementation of the visualization. More additions are planned, but would take too much time/attention at the moment (see AI4S2S/lilio#6 ).


If multiple anchor years are to be displayed, the plot looks like the following:
image


With the relative_dates kwarg, the calendar can either be aligned to the anchors or show absolute dates of the intervals:
image

@BSchilperoort BSchilperoort marked this pull request as draft September 26, 2022 11:50
@BSchilperoort
Copy link
Contributor Author

Note that the linter fails only because of a known mypy bug (python/mypy#13627)

@BSchilperoort BSchilperoort marked this pull request as ready for review September 26, 2022 13:56
@Peter9192
Copy link
Contributor

Note that the linter fails only because of a known mypy bug (python/mypy#13627)

Can we suppress the linter for that line, and add a comment to the bug report? Ideally, we should avoid accepting red CI, orwe might get sloppy and stop checking it because it wasn't green to start with.

@BSchilperoort BSchilperoort removed the request for review from geek-yang September 27, 2022 15:05
@BSchilperoort BSchilperoort marked this pull request as draft September 27, 2022 15:05
@BSchilperoort
Copy link
Contributor Author

Working on some refactoring and streamlining of the code, along with adding your (live) suggestions 👍

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -272,6 +272,62 @@
"larger_calendar.visualize(add_freq=True)"
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be fixed. It runs well locally on my machine. I think you haven't updated/run this notebook after changing your code. It must be from .bokeh_plots import bokeh_visualization , which is correct in your code. Just run it again.


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this seems to have gone wrong. Thanks for noticing!

Copy link
Member

@geek-yang geek-yang left a comment

Choose a reason for hiding this comment

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

Nice work! The implementation is also quite elegant. Only for the file plot.py, I think the user doesn't need it and it can also be a hidden thingy, just like _bokeh_plots.py.

Changes are approved. Just feel free to merge it to main after addressing the minor comments. Thanks for the nice work!

n_years = min(n_years, len(self.get_intervals().index))
plot.matplotlib_visualization(self, n_years, add_freq)

def visualize_bokeh(
Copy link
Member

Choose a reason for hiding this comment

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

visualize_bokeh sounds a bit implicit, especially for the user not familiar with bokeh. I prefer a more explicit method name, like visualize_interactive.

Suggested change
def visualize_bokeh(
def visualize_interactive(

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

85.6% 85.6% Coverage
0.0% 0.0% Duplication

@BSchilperoort BSchilperoort merged commit 40475a6 into main Sep 29, 2022
@BSchilperoort BSchilperoort deleted the bokeh_test branch September 29, 2022 07:23
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