-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Allow disabling lookup of missing prop names #4100
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
Comments
Oof, this is a complex call stack. From your comment in the other issue "it is pre-calculating an error message for a missing key" ... it seems to me like pre-calculating the messages is the problem here. If we're not actually using any invalid keys, we shouldn't be anywhere near a levenshtein calculation IMO. Since you've been in the code here, does there seem like a decent way to not precalculate things? |
I think the top of |
When running a large update on a plot, sometimes the time is dominated by the lookup for missing names, even when this lookup serves no purpose, such as in
plotly.subplots.make_subplots
where new axes are being added by the plotly library itself, and finding "nearby" names is not helpful when all that is needed is a check if the name already exists.In
plotly.basedatatypes.BaseDatatype._perform_update
, for each key to be updated it calls_check_path_in_prop_tree
, but then ignores the error that is generated if theplotly_obj
is aBaseLayoutType
and the key matches one of the_subplot_re_match
keywords, and just adds the key to the plotly_obj instead.This call to
_check_path_in_prop_tree
is expensive whenWhen those conditions are met, the prop lookup
check_path_in_prop_tree
callsBasePlotlyType.__getitem__
, which then callsBasePlotlyType._raise_on_invalid_property_error
, which then calls_plotly_utils.utils.find_closest_string
for each new property being added.find_closest_string
uses alevenshtein
lookup and this takes geometrically more time the more keys there are to compare, which is the root of the problem.If the lookup could be disabled in
_raise_on_invalid_property_error
, that would help, but there is a problem in that_check_path_in_prop_tree
is using an implicit__getitem__
on theplotly_obj
, hereThe issue can be resolved by disabling the lookup completely, as shown in the second example notebook below, where
_plotly_utils.utils
is monkey-patched so thatfind_closest_string
always raises an error, but this doesn't seem like the best way to handle it.Example one: without disabling lookup
Example two: disabling lookup
The text was updated successfully, but these errors were encountered: