-
Notifications
You must be signed in to change notification settings - Fork 26
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
feature request: save Report.comparisons as JSON #4
Comments
Hi, an export option for the Report class is already on my to-do list! :) I will come back with a proposal so that we can discuss it before I implement the functionality. |
Hey, sorry for the delay. This is my proposal for the {
# metrics and model_names allows to read the report without
# inspecting the json to discover the used metrics and
# the compared models
"metrics": ["metric_1", "metric_2", ...],
"model_names": ["model_1", "model_2", ...],
#
"model_1": {
"scores": {
"metric_1": ...,
"metric_2": ...,
...
},
"comparisons": {
"model_2": {
"metric_1": ..., # p-value
"metric_2": ..., # p-value
...
},
...
},
"win_tie_loss": {
"model_2": {
"W": ...,
"T": ...,
"L": ...,
},
...
},
},
...
} Let me know what you think. :) |
Looks great (and there was not so much delay 😅)! |
I added Closing. |
I’m getting a "TypeError: Object of type int64 is not JSON serializable" which is probably coming from numba or numpy |
Yeah, I know about that issue. I will look into it soon. As a workaround, you can call That issue it's kinda weird. |
don’t you need to convert |
for example in def denumpify_detensorize(metrics):
"""
Recursively calls `.item()` on the element of the dictionary passed
"""
if isinstance(metrics, (list, tuple)):
return type(metrics)(denumpify_detensorize(m) for m in metrics)
elif isinstance(metrics, dict):
return type(metrics)({k: denumpify_detensorize(v) for k, v in metrics.items()})
elif isinstance(metrics, np.generic):
return metrics.item()
elif is_torch_available() and isinstance(metrics, torch.Tensor) and metrics.numel() == 1:
return metrics.item()
return metrics |
this fixes it but you probably want to deal with it some other way? If not I can open a PR PaulLerner@7e2218d |
I will look into it soon. |
Fixed in |
I’m still getting |
oops, looks like I was on the wrong branch, sorry about that |
Hi,
It’d be nice to be able to save a Report comparisons as a JSON file.
However, since it uses frozenset as keys, it is not JSON serializable.
Maybe you could add a method in https://github.com/AmenRa/ranx/blob/master/ranx/frozenset_dict.py to convert the
_map
to a JSON serializable dict, i.e. withstr
keys?The
str
keys could be converted from the frozenset like:', '.join(frozenset({'foo', 'bar'}))
The text was updated successfully, but these errors were encountered: