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

Adding the baselines results csvs and data #25

Merged
merged 8 commits into from Aug 27, 2021
Merged

Conversation

megstanley
Copy link
Contributor

@megstanley megstanley commented Aug 27, 2021

Adding baselines summary csvs that are used by notebooks/visualize to plot everything. Update this PR 'ed branch with final PN results when available.

Also small changes to plotting utils to allow consistent task highlighting/less naff colours etc.

also moved the target_info csvs

Copy link
Contributor

@kmaziarz kmaziarz left a comment

Choose a reason for hiding this comment

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

This seems to delete the csvs in preprocessing/targets (and not move them). Is that intended?

Comment on lines 984 to 987
# colors = [200, 128, 64, 160, 10, 32]
# color_set = [plt.get_cmap("plasma").colors[x] for x in colors]
# colors = [175, 170, 160, 64, 32, 10]
# color_set = [plt.get_cmap("nipy_spectral")(x) for x in colors]#
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop all this?

for i, model_name in enumerate(model_summaries.keys()):
color = plt.get_cmap("plasma").colors[i * 50 + 50]
for j, model_name in enumerate(model_summaries.keys()):
# color = plt.get_cmap("plasma").colors[j * 50 + 40]
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this line too?

@megstanley
Copy link
Contributor Author

This seems to delete the csvs in preprocessing/targets (and not move them). Is that intended?

Yes, they are put in datasets/targets in another PR

Copy link
Contributor

@kmaziarz kmaziarz left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe also edit the PR title to include the fact that this also adds the data 🙂

@megstanley megstanley changed the title Adding the baselines results csvs Adding the baselines results csvs and data Aug 27, 2021
@megstanley megstanley merged commit fa336ae into main Aug 27, 2021
@mmjb mmjb deleted the feat/datasets branch August 27, 2021 21:18
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.

None yet

2 participants