Skip to content
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

Contigency table visual representation for notebooks #775

Merged
merged 8 commits into from
Dec 5, 2024

Conversation

arshiaar
Copy link
Contributor

Please work through the following checklists. Delete anything that isn't relevant.

Development for new xarray-based metrics

  • [x ] Typehints added

Docstrings

  • Docstrings complete and follow Napoleon (google) style
  • Code example added

Tutorial notebook

-[x] update tutorial notebook

@arshiaar
Copy link
Contributor Author

#361

@tennlee tennlee linked an issue Nov 26, 2024 that may be closed by this pull request
@tennlee
Copy link
Collaborator

tennlee commented Dec 1, 2024

Hi @arshiaar. We had discussed some adjustments to the dataframe labels. Can you please indicate if you plan to continue this PR to implement them? If not, that is completely fine, I will happily progress the work from here, but I wanted to give you the chance to let us know if you'd prefer to do this yourself.

@tennlee tennlee force-pushed the feature/contigency_table_visual branch from 446a254 to 7144710 Compare December 2, 2024 10:37
@tennlee
Copy link
Collaborator

tennlee commented Dec 2, 2024

I have rebased this work (brought it up to date against the 'develop' branch) and pushed up that change into this PR.

Modify column and row labels for HTML rendered contingency table
Update contigency table tutorial to match.
@tennlee
Copy link
Collaborator

tennlee commented Dec 2, 2024

@nicholasloveday This PR is now at a review-ready state.

@tennlee
Copy link
Collaborator

tennlee commented Dec 2, 2024

@arshiaar I removed the _repr_html method. As a side note, it needs to be named repr_html to be recognised by JupyterLab. However, the function doesn't work in all cases. If any dimensionality has been preserved, the confusion matrix is no longer 2x2. As such, I thought it was perhaps not ideal to have as default rendering function. I'm not entirely sure how higher-dimensional confusion matrixes should be handled, so I just removed it as a default renderer and put a note in the docstring, which I think is sufficient. I think a lot of the use cases will still be met, and this is a nice representation of the most common 2x2 case. Thanks again for all your help.

@nicholasloveday
Copy link
Collaborator

This looks to work nicely for the 2x2 case.

For higher dimensions, people can do something like

new_manager = contingency_manager.transform(preserve_dims='lead_hour')
new_manager.get_table()

It doesn't display a nice table, but it people can access the data and make their own tables.

One thing that would be nice to do is to provide a more useful error message if you call format_table on a higher dimensional matrix

new_manager = contingency_manager.transform(preserve_dims='lead_hour')
new_manager.format_table()

@tennlee
Copy link
Collaborator

tennlee commented Dec 5, 2024

Thanks Nick. I had been wondering about whether to go to the extra effort to provide some more error-handling, and in light of your comments I will do so, and post here when I have something implemented.

@tennlee
Copy link
Collaborator

tennlee commented Dec 5, 2024

@nicholasloveday I have updated the PR with some additional handling code - what do you think?

@nicholasloveday
Copy link
Collaborator

Thanks - the update looks good to me!

@tennlee tennlee marked this pull request as ready for review December 5, 2024 05:42
@tennlee tennlee changed the title contigency table visual init commit Contigency table visual representation for notebooks Dec 5, 2024
@tennlee tennlee merged commit 53a39c9 into nci:develop Dec 5, 2024
6 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.

Improved visual rendering of contingency tables
3 participants