Avoid ValueError when comparing embed defs with NumPy arrays#7980
Avoid ValueError when comparing embed defs with NumPy arrays#7980mscolnick merged 4 commits intomarimo-team:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
| def test_numpy_defs_do_not_crash_and_invalidate_cache(): | ||
| runner = _make_runner() | ||
|
|
||
| import numpy as np |
There was a problem hiding this comment.
we will need to skip these tests if numpy is not installed.
you can do @pytest.mark.requires("numpy") on the test function
There was a problem hiding this comment.
Thanks for the feedback! I’ve updated the code as requested.
marimo/_runtime/app/kernel_runner.py
Outdated
|
|
||
| try: | ||
| # numpy-safe comparison | ||
| try: |
There was a problem hiding this comment.
can you do if DependencyManager.numpy.imported() before this so we don't import numpy if it hasnt been imported (otherwise this could be a slower operation
There was a problem hiding this comment.
I’ve updated the implementation to use DependencyManager.numpy as suggested.
|
@MukeshK17 there is a small typecheck error
|
|
Fixed the reported typecheck issue. |
Fix embed caching when defs contain NumPy arrays
Summary
Fixes a bug where
App.embed()could raise aValueErrorwhen embeddefscontain NumPy arrays.The issue occurred during cache checks when comparing
defsusing==, which is ambiguous for NumPy arrays.Problem
Changing embed
defscontaining NumPy arrays (e.g. arrays with different shapes) could raise:ValueError: The truth value of an array with more than one element is ambiguousThis originated in
AppKernelRunner.are_outputs_cached.Solution
_defs_equal) for comparing embeddefsnumpy.array_equalfor NumPy arraysTests
Added regression tests ensuring:
These tests fail on
mainand pass with this change.Fixes #7969
CLA
I have read the CLA Document and I hereby sign the CLA.