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

Fix bokeh 3.4.0 deprecation warnings for circle(), cross() #1428

Merged

Conversation

orionlee
Copy link
Collaborator

@orionlee orionlee commented May 13, 2024

Close #1424

Manually tested on bokeh 3.4.1 and bokeh 2.4.3 . No regression found.

changelog:

- Fixed interact features, e.g. ``tpf.interact()``, to be compliant
  with Bokeh v3.4.0. The minimum Bokeh version is raised to v2.3.2 accordingly. [#1428]

@orionlee
Copy link
Collaborator Author

@Nschanche A small fix that has no surprise. I intend to merge this PR in the next few days.

@Nschanche
Copy link
Collaborator

The required bokeh version for lightkurve appears to be 2.0.0. Adding multiple marker types seems to have been added to bokeh in 2.4.3, so we should probably change the requirement in the toml file before merging.

@orionlee
Copy link
Collaborator Author

orionlee commented May 14, 2024

Good catch! The minimum bokeh requirement is raised to 2.3.2.
tpf.interact(), tpf.interact_sky() and lc.interact_bls() were manually tested.

  • the scatter with marker api is available from v2.3.0
  • but it turns out that there is a bigger issue with old bokeh, which uses the (now) removed jinja2.Markup, causing import failure of bokeh show_notebook(). It is fixed in v2.3.2,

An alternative is to raise the minimum bokeh requirement to 2.4.3, given it is the oldest release that still has documentation readily available.

…nose problems

- e.g., if the user has an old version of bokeh that is incompatible with others, such as jinja2
@orionlee orionlee force-pushed the fix_bokeh_340_deprecation_circle_etc_1424 branch from 980eb8f to 47cb3d2 Compare May 14, 2024 17:02
@orionlee
Copy link
Collaborator Author

orionlee commented May 14, 2024

An additional commit is made to tweak how lightkurve reports bokeh import issues. In addition to emitting a friendly message, it now also throws the actual import error, to assist users to diagnose the problem.


Currently, if there is any error during bokeh import, lightkurve will silently consume the error, assuming bokeh is not installed, and emit an error message asking users to install bokeh when the relevant function is called, e.g., tpf.interact().

It could lead to confusion when users do have bokeh installed, but there is some other issues during import.

E.g., if users have old bokeh v2.3.0 installed along with some recent version of jinja2 (the two are not compatible), calling tpf.interact() would fail with a head-scratching NameError:

-> 1347 output_notebook(verbose=False, hide_banner=True)
   1348 return show(create_interact_ui, notebook_url=notebook_url)

NameError: name 'output_notebook' is not defined

The commit changes it so that it displays the actual import error when calling tpf.interact().
The import error can be served as hints that there is some incompatibility between bokeh and jinja2 in the installation.

The interact() tool requires the `bokeh` Python package; you can install bokeh using e.g. `conda install bokeh`.
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
Cell In[2], line 10
...
...
File C:\pkg\_winNonPortables\miniforge3\envs\lk_dev\lib\site-packages\bokeh\core\templates.py:42
     39 from os.path import dirname, join
     41 # External imports
---> 42 from jinja2 import Environment, FileSystemLoader, Markup
     44 #-----------------------------------------------------------------------------
     45 # Globals and constants
     46 #-----------------------------------------------------------------------------
     48 __all__ = (
     49     'AUTOLOAD_JS',
     50     'AUTOLOAD_NB_JS',
   (...)
     63     'SCRIPT_TAG',
     64 )

ImportError: cannot import name 'Markup' from 'jinja2' (C:\pkg\_winNonPortables\miniforge3\envs\lk_dev\lib\site-packages\jinja2\__init__.py)

@orionlee orionlee merged commit 05a20b5 into lightkurve:main May 17, 2024
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.

tpf.interact() and lc.interact_bls(): handle bokeh deprecation of circle() starting v3.4.0
2 participants