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

jupyter notebook for heatmap snippet, with three options #6

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

opotowsky
Copy link
Member

@opotowsky opotowsky commented Jun 8, 2020

This snippet is for CNERG folk who want to plot 2D heatmaps given x, y, z experimental data. This uses triangulation to form a triangular grid and then applies matplotlib's tricontourf (or tripcolor) to get the surface plot with a colorbar

@opotowsky
Copy link
Member Author

I'm not sure how to request to merge this branch into a branch in the CNERG repo that does not yet exist.

@opotowsky opotowsky requested a review from ypark234 June 8, 2020 20:05
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

I'm not sure if we've finalized the layout of this repo, but instead of PR'ing to another branch, I would recommend PR'ing to master but with this code in its own directory.

When doing so, also:

  • updated the README to describe it better
  • the notebook fails when started as-is - I think the data needs the random generation rather than the arange

@opotowsky opotowsky changed the title initial jupyter notebook for heatmap snippet, with three options jupyter notebook for heatmap snippet, with three options Jun 15, 2020
@gonuke
Copy link
Member

gonuke commented Jun 16, 2020

Thanks for this addition @opotowsky. I think this approach is particularly useful when your data is on an irregular X/Y grid. There are probably simpler methods for a regular grid.

@opotowsky
Copy link
Member Author

Do I need to worry about configuring anything with CircleCI?

@gonuke
Copy link
Member

gonuke commented Jun 16, 2020

CircleCI is not ready yet. I am not sure if we have a policy on merging before we get that done, or if we want someone to tackle that first???

@kkiesling
Copy link
Member

We need someone to tackle CI before we can (or should) merge anything. I think @bam241 was going to do this (#3)? (not to put more on your plate..) @ypark234 's PR is still waiting too (#1).

@opotowsky
Copy link
Member Author

Great, thanks for catching me up!

Copy link
Contributor

@ypark234 ypark234 left a comment

Choose a reason for hiding this comment

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

Thanks for follow-ups on this topic, @opotowsky !
Took longer than I thought to revisit #3 as it was not my top priority..
Hopefully #3 gets merged successfully soon and then we can think about what to do with CI on this PR.

@ypark234 ypark234 changed the base branch from master to main June 18, 2020 21:07
@bam241
Copy link
Member

bam241 commented Jun 24, 2020

@opotowsky could you rebase against main ?

it should trigger the CI.

@opotowsky
Copy link
Member Author

Thanks @bam241 ...done!

Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

LGTM - thanks @opotowsky

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.

5 participants