Skip to content

Conversation

@roshankern
Copy link
Member

This PR is the first of many to restructure the evaluate model module and output the evaluations in tidy long format for later figure generation.

In this PR, a separate notebook is created to display the confusion matrices for the final and shuffled baseline models' predictions for the train and test datasets. These confusion matrices are converted to tidy long format and saved in confusion matrices.

roshankern and others added 14 commits December 9, 2022 15:51
* finish download module changes

* download notebook

* rerun split data module

* rerun download module

* rerun train_model

* rerun evaluation module

* rerun interpretation module

* combine datasets

* combine datasets

* split changes

* update format

* format update

* format

* finish split data

* combine datasets, remove holdout

* formatting

* rerun pipelines

* remove folded class

* rerun pipeline

* Update utils/download_utils.py

Co-authored-by: Dave Bunten <ekgto445@gmail.com>

* PR fixes

* module docstrings

Co-authored-by: Dave Bunten <ekgto445@gmail.com>
@roshankern roshankern requested a review from d33bs February 6, 2023 14:56
Copy link
Member

@d33bs d33bs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I left a few comments with this review - please don't hesitate to reach out if you have any questions.

@roshankern roshankern merged commit c5c6542 into WayScience:main Feb 6, 2023
@roshankern roshankern deleted the refactor-evaluate-module branch February 6, 2023 22:02
Copy link
Member

@gwaybio gwaybio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

return fig, PR_data


def model_cm(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rename this model_confusion_matrix() it is clearer than cm

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! Completed in #8 for simplicity.

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.

3 participants