Skip to content

Conversation

@gudlot
Copy link
Collaborator

@gudlot gudlot commented Nov 19, 2025

Calibration Notebook and widgets Update

Summary of Changes

  • Calibration Widget Fix

    • Corrected the import of ipywidgets in pyFAI/gui/jupyter/calib.py to ensure the widgets namespace is always defined.
    • Adjusted the exception handling around IPython.display and ipywidgets imports for better compatibility in different environments (VS Code, JupyterLab, Jupyter Notebook).
  • Notebook Cleanup

    • Removed unnecessary strings calibration_with_jupyter.ipynb.
  • Behavior

    • The notebook executes fully, including interactive widget-based calibration, while maintaining outputs (as preferred for tutorials).

Notes

  • The notebook and widget functionality have been tested in VS Code; full autoplay video/audio may still require JupyterLab or a browser environment.

Copy link
Member

Choose a reason for hiding this comment

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

Apparently, putting widgets outside of the try/except block breaks the tests on windows ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did first pip install . in my env. Then I tried to run the notebook, but it failed.
The cell starting with

%matplotlib widget
calib = Calibration(img, calibrant=AgBh, wavelength=wavelength, detector=pilatus)

failed with an error in calib.py in line 22 self.mplw = widgets.Output()

I just run python run_tests.py (without -xo) again:

Ran 569 tests in 306.110s

OK (skipped=48)

With -xo

----------------------------------------------------------------------
Ran 495 tests in 248.661s

OK (skipped=20)

Do you mean the tests test_bug_2538 until test_sort4?

Then I have to go back and figure out another solution.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kif I would like to make the following suggestion for calib.py

try:
    from IPython.display import display
    import ipywidgets as widgets
except (ImportError, ModuleNotFoundError):
    try:
        # for compatibility with older Python
        from IPython.core.display import display
        import ipywidgets as widgets
    except (ImportError, ModuleNotFoundError):
        from ...utils.callback import dangling_callback as display
  • IPython.core.display replaced by IPython.display for modern IPython.
  • Fallback kept for backward compatibility.
  • Catch now (ImportError, ModuleNotFoundError) instead of a bare except.

https://ipython.readthedocs.io/en/stable/whatsnew/version7.html#pending-deprecated-imports

The test statistics are the same with this snippet with python run_tests.py

----------------------------------------------------------------------
Ran 569 tests in 295.597s
OK (skipped=48)

Copy link
Member

Choose a reason for hiding this comment

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

Why did you add the PONI-file to the test ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought you might prefer an updated file that fits the outcome of the ipynb. But I can add *.poni to my .gitignore.

Copy link
Member

Choose a reason for hiding this comment

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

It is just that the notebook is re-run at each release and it makes no sense to update the file as it comes from a least squares refinement of randomly selected points: not deterministic.

Added import for ipywidgets to enhance Jupyter compatibility.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Member

@kif kif left a comment

Choose a reason for hiding this comment

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

The modification on the notebook look great.

@kif kif merged commit 7c0c9b8 into silx-kit:main Nov 21, 2025
8 checks passed
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.

2 participants