I1673 add support for Nan values in the N2 diagram #1677
Conversation
| @@ -77,7 +94,7 @@ def _get_var_dict(system, typ, name): | |||
| var_dict['value'] = type(meta['value']).__name__ | |||
| else: | |||
| if meta['value'].size < _MAX_ARRAY_SIZE_FOR_REPR_VAL: | |||
| var_dict['value'] = meta['value'] | |||
| var_dict['value'] = _convert_to_support_nans_in_json(meta['value']) | |||
| else: | |||
There was a problem hiding this comment.
I think you can probably add a test for this if calling n2 on a prob with NaN formerly raised an exception. Just do n2(p, show_browser=False) so that it doesn't open the browser or write a file.
There was a problem hiding this comment.
Great point. I had that in my head as a to do yesterday and then forgot today. Will add
There was a problem hiding this comment.
similar to other comment..
I think you could replace this single use function with something like:
var_dict['value'] = _convert_nans_in_nested_list(meta['value'].tolist())
| val_string = val_float_formatter(element); | ||
| } | ||
| } else { | ||
| val_string = stringify_with_support_for_nan(element); |
There was a problem hiding this comment.
this function is only used in this one place
since it's a simple if/else, I think you could just inline it and avoid a function call
| @@ -77,7 +94,7 @@ def _get_var_dict(system, typ, name): | |||
| var_dict['value'] = type(meta['value']).__name__ | |||
| else: | |||
| if meta['value'].size < _MAX_ARRAY_SIZE_FOR_REPR_VAL: | |||
| var_dict['value'] = meta['value'] | |||
| var_dict['value'] = _convert_to_support_nans_in_json(meta['value']) | |||
| else: | |||
There was a problem hiding this comment.
similar to other comment..
I think you could replace this single use function with something like:
var_dict['value'] = _convert_nans_in_nested_list(meta['value'].tolist())
Summary
The N2 diagram did not handle the case where initial values had Nan values in them.
Also did some re-factoring of related JavaScript code to reduce code duplication.
Related Issues
Backwards incompatibilities
None
New Dependencies
None