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.

Sign up for GitHub

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

Jump to bottom

Add histogram gallery example #1272

Merged
merged 16 commits into from
May 18, 2021
Merged

Add histogram gallery example #1272

merged 16 commits into from
May 18, 2021

Conversation

michaelgrund
Copy link
Member

@michaelgrund michaelgrund commented May 12, 2021 •
edited by weiji14
Loading

Description of proposed changes

Since histogram is now wrapped in #1072, here's a corresponding gallery example.

Preview at https://pygmt-git-gallery-histogram-gmt.vercel.app/gallery/histograms/histogram.html

Histogram gallery example

I guess we should also add the v3206_06.txt remote file to the cache data, right?

This is continuation of work on #563.

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@michaelgrund michaelgrund added the documentation Improvements or additions to documentation label May 12, 2021
@michaelgrund michaelgrund added this to the 0.4.0 milestone May 12, 2021
@michaelgrund michaelgrund marked this pull request as draft May 12, 2021 16:42
@michaelgrund michaelgrund marked this pull request as ready for review May 12, 2021 17:03
@seisman
Copy link
Member

seisman commented May 13, 2021

I guess we should also add the v3206_06.txt remote file to the cache data, right?

Perhaps we should just generate some random data like this one https://matplotlib.org/stable/gallery/statistics/histogram_features.html.

@michaelgrund
Copy link
Member Author

I guess we should also add the v3206_06.txt remote file to the cache data, right?

Perhaps we should just generate some random data like this one https://matplotlib.org/stable/gallery/statistics/histogram_features.html.

Of course, would be also fine for me.

seisman
seisman reviewed May 13, 2021
View reviewed changes
examples/gallery/histograms/histogram.py Outdated Show resolved Hide resolved
examples/gallery/histograms/histogram.py Outdated Show resolved Hide resolved
@seisman
Copy link
Member

seisman commented May 13, 2021 •
edited
Loading

The example looks good to me, but I'm wondering if we should change the parameter type to histtype, because type is a Python's built-in method.

FYI, GMT.jl uses kind and matplotlib uses histtype.

Ping @willschlitzer

@michaelgrund
Copy link
Member Author

michaelgrund commented May 14, 2021 •
edited
Loading

The example looks good to me, but I'm wondering if we should change the parameter type to histtype, because type is a Python's built-in method.

FYI, GMT.jl uses kind and matplotlib uses histtype.

Ping @willschlitzer

Agree with @seisman since type is always highighted as Python built-in method in Jupyter as well as any IDE. Will open an separate PR to change it.

michaelgrund added a commit that referenced this pull request May 14, 2021
@michaelgrund
Rename Z alias in histogram
0f912b3 
As discussed in #1272 the alias for Z should be renamed since ``type`` is already used as Python built-in method.
@michaelgrund michaelgrund mentioned this pull request May 14, 2021
Merged
5 tasks
weiji14
weiji14 reviewed May 15, 2021
View reviewed changes
examples/gallery/histograms/histogram.py Outdated Show resolved Hide resolved
examples/gallery/histograms/histogram.py Outdated Show resolved Hide resolved
examples/gallery/histograms/histogram.py Outdated Show resolved Hide resolved
michaelgrund
michaelgrund commented May 15, 2021
View reviewed changes
Copy link
Member Author

@michaelgrund michaelgrund 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 the comments and suggestions @weiji14.

@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label May 15, 2021
seisman
seisman approved these changes May 15, 2021
View reviewed changes
Copy link
Member

@seisman seisman left a comment

Choose a reason for hiding this comment

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

Looks good to me.

willschlitzer
willschlitzer approved these changes May 16, 2021
View reviewed changes
Copy link
Contributor

@willschlitzer willschlitzer 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!

core-man
core-man approved these changes May 17, 2021
View reviewed changes
Copy link
Member

@core-man core-man left a comment

Choose a reason for hiding this comment

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

Looks great~

weiji14
weiji14 approved these changes May 18, 2021
View reviewed changes
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

🚀

@weiji14 weiji14 self-requested a review May 18, 2021 11:25
@weiji14 weiji14 merged commit 5125224 into master May 18, 2021
@weiji14 weiji14 deleted the gallery-histogram branch May 18, 2021 11:32
@weiji14 weiji14 removed the final review call This PR requires final review and approval from a second reviewer label May 23, 2021
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
@michaelgrund @seisman
@weiji14 @willschlitzer
Add histogram gallery example (GenericMappingTools#1272)
3266e4c 
Gallery example of a histogram with red bars on a gray background.
Elevation counts are binned at 5m intervals, and the data is
generated from a normal distribution with random noise added.

Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Will Schlitzer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@core-man core-man core-man approved these changes

@willschlitzer willschlitzer willschlitzer approved these changes

@seisman seisman seisman approved these changes

@weiji14 weiji14 Awaiting requested review from weiji14

Assignees

@michaelgrund michaelgrund

Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
0.4.0
Development

Successfully merging this pull request may close these issues.
5 participants
@michaelgrund @seisman @weiji14 @willschlitzer @core-man