Improvement of two gallery examples regarding categorical colormaps by yvonnefroehlich · Pull Request #1934 · GenericMappingTools/pygmt

yvonnefroehlich · 2022-05-26T18:09:08Z

Description of proposed changes

Fixes #1933

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 wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
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

welcome · 2022-05-26T18:09:10Z

💖 Thanks for opening this pull request! 💖

Please make sure you read our contributing guidelines and abide by our code of conduct.

A few things to keep in mind:

If you need help writing tests, take a look at the existing ones for inspiration. If you don't know where to start, let us know and we'll walk you through it.
All new features should be documented. It helps to write the docstrings for your functions/classes before writing the code. This will help you think about your code design and results in better code.
No matter what, we are really grateful that you put in the effort to do this! 🎉

yvonnefroehlich · 2022-05-27T19:32:45Z

I altered the code as suggested by @seisman in #1933 comment.

yvonnefroehlich · 2022-05-27T19:32:54Z

I am very sorry for accidentally closing this PR! I reopened it.

seisman · 2022-05-28T16:28:02Z

Ping @michaelgrund and @weiji14 for reviewing this PR, as they wrote the initial version of the example.

examples/gallery/3d_plots/scatter3d.py

weiji14

Thanks for taking up this Pull Request @yvonnefroehlich!

I've made a few suggested changes to the documentation wording, which you're welcome to "Commit suggestion" directly, or pool several together using "Add suggestion to batch" before committing. See also https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/incorporating-feedback-in-your-pull-request#applying-a-suggested-change.

weiji14 · 2022-06-01T12:50:47Z

examples/gallery/3d_plots/scatter3d.py

 pygmt.makecpt(
-    cmap="cubhelix", color_model="+cSetosa,Versicolor,Virginica", series=(0, 2, 1)
+    cmap="cubhelix",
+    # Use the minum and maximum of the categorical number code


Suggested change

# Use the minum and maximum of the categorical number code

# Use the minimum and maximum of the categorical values (alphabetically sorted)

Pandas assigns numbers (in an alphabetical manner) to the individual categories, when converting df.species to the categorical data type:

df.species = df.species.astype(dtype="category")

Color points by categories: Adelie -> 0, Chinstrap -> 1, Gentoo -> 2

3D Scatter plots: setosa -> 0, versicolor -> 1, virginica -> 2

These numbers are used to define the range of the colormap (series of pygmt.makecpt):

series=(df.species.cat.codes.min(), df.species.cat.codes.max(), 1),

Thus, I think we should use here 'categorical number code'. Also because this term is later used in the comment for the color parameter of the pygmt.Figure.plot() or pygmt.Figure.3Dplot() method:

# Points colored by categorical number code color=df.species.cat.codes.astype(int),

I'm not that familiar with panda's categorical data type, but looking at the documentation and the functions calls df.species.cat.categories and df.species.cat.codes, I think categories and number code are better.

Hmm you're right, let's go with categories and number code then. Wasn't aware that pandas actually uses numbers for categorical data!

examples/gallery/3d_plots/scatter3d.py

weiji14 · 2022-06-01T12:56:45Z

examples/gallery/symbols/points_categorical.py

+# By default pandas assigns the categorical number code in an alphabetical
+# manner to the individual categories. For a non-alphabetical order, you have
+# to adjust the categorical number code. For handling and manipulating
+# categorical data in pandas you may have a look at:
+# https://pandas.pydata.org/docs/user_guide/categorical.html


Suggested change

# By default pandas assigns the categorical number code in an alphabetical

# manner to the individual categories. For a non-alphabetical order, you have

# to adjust the categorical number code. For handling and manipulating

# categorical data in pandas you may have a look at:

# https://pandas.pydata.org/docs/user_guide/categorical.html

# By default, pandas sorts the individual categorical values in an alphabetical

# order. For a non-alphabetical order, you have to manually adjust the list of

# categorical values. For handling and manipulating categorical data in pandas,

# have a look at:

# https://pandas.pydata.org/docs/user_guide/categorical.html

There is a typo in the suggested change in line 21: you instead of ou.

Good spotting, I've corrected the suggested change.

weiji14 · 2022-06-01T12:57:17Z

examples/gallery/symbols/points_categorical.py

-pygmt.makecpt(cmap="inferno", series=(0, 2, 1), color_model="+cAdelie,Chinstrap,Gentoo")
+pygmt.makecpt(
+    cmap="inferno",
+    # Use the minum and maximum of the categorical number code


Suggested change

# Use the minum and maximum of the categorical number code

# Use the minimum and maximum of the categorical values (alphabetically sorted)

yvonnefroehlich · 2022-06-07T16:58:15Z

Thanks for taking up this Pull Request @yvonnefroehlich!

I have to say thank you for the nice and welcoming meeting by @meghanrjones and @weiji14 during the EGU conference! 😊

Also thanks to @seisman and @weiji14 for their reviews!
But I am a bit unsure how to continue. Should I submit a commit for fixing the typo and interpunction as well as shortening the text (corresponding to the review by @weiji14), but keeping ‘categorical number code’ and ‘categories’ (corresponding to the comment by @seisman)? Or will there be a new review by @weiji14? Or should we wait for the review by @michaelgrund?

weiji14 · 2022-06-07T18:23:45Z

Also thanks to @seisman and @weiji14 for their reviews! But I am a bit unsure how to continue. Should I submit a commit for fixing the typo and interpunction as well as shortening the text (corresponding to the review by @weiji14), but keeping ‘categorical number code’ and ‘categories’ (corresponding to the comment by @seisman)? Or will there be a new review by @weiji14? Or should we wait for the review by @michaelgrund?

On second thought, let's keep using 'categories' or `number code' as suggested by @seisman in #1934 (comment) (sorry for the confusion, I was a bit mistaken with pandas categorical data). If @michaelgrund has time, it would be nice to get his input since he wrote this example originally, but if not, we can still continue with the Pull Request.

yvonnefroehlich · 2022-06-08T05:27:14Z

On second thought, let's keep using 'categories' or `number code' as suggested by @seisman in #1934 (comment) (sorry for the confusion, I was a bit mistaken with pandas categorical data). If @michaelgrund has time, it would be nice to get his input since he wrote this example originally, but if not, we can still continue with the Pull Request.

No worries and no hurry @weiji14 😉
I am totally fine with waiting for the review by @michaelgrund before pushing a further commit.

michaelgrund · 2022-06-08T07:59:27Z

I'm fine with all the changes made so far!

yvonnefroehlich · 2022-06-08T20:11:52Z

I'm fine with all the changes made so far!

Thanks for your response @michaelgrund !

I updated the text based on the reviews and discussion.

seisman

Looks great!

weiji14

Nice one, thanks again @yvonnefroehlich!

welcome · 2022-06-09T02:03:41Z

🎉🎉🎉 Congrats on merging your first pull request and welcome to the team! 🎉🎉🎉

Please open a new pull request to add yourself to the AUTHORS.md file. We hope that this was a good experience for you. Let us know if there is any way that the contributing process could be improved.

yvonnefroehlich · 2022-06-10T20:41:26Z

Nice one, thanks again @yvonnefroehlich!

I have to say thank you for allowing and helping me to work on this issue / pull request! 😊

We hope that this was a good experience for you. Let us know if there is any way that the contributing process could be improved.

Definitely! 😊 - Likely I learned more than I brought to the project! 😉

…enericMappingTools#1934) Clarify that by default the labels for the colorbar must be given in alphabetical order. * add remark regarding categorical number code * use flexible arguments in makecpt * shorten and replace remark regarding categorical number code * fix typos and shorten text based on review

add remark regarding categorical number code

0d0749e

vercel bot temporarily deployed to Preview May 26, 2022 18:18 Inactive

yvonnefroehlich mentioned this pull request May 27, 2022

Suggestion for improvement of gallery examples regarding categorical colormaps #1933

Closed

use flexible arguments in makecpt

7bcea08

yvonnefroehlich closed this May 27, 2022

yvonnefroehlich reopened this May 27, 2022

vercel bot temporarily deployed to Preview May 27, 2022 19:33 Inactive

seisman requested review from michaelgrund and weiji14 May 28, 2022 16:28

seisman added the documentation Improvements or additions to documentation label May 28, 2022

seisman added this to the 0.7.0 milestone May 28, 2022

seisman reviewed May 31, 2022

View reviewed changes

examples/gallery/3d_plots/scatter3d.py Outdated Show resolved Hide resolved

seisman reviewed May 31, 2022

View reviewed changes

examples/gallery/3d_plots/scatter3d.py Show resolved Hide resolved

yvonnefroehlich added 2 commits June 1, 2022 09:14

shorten and replace remark regarding categorical number code

f2533ae

replace tabs by spaces

58210a2

yvonnefroehlich changed the title ~~Add remark regarding pandas categorical number code in two gallery examples~~ Improvement of two gallery examples regarding categorical colormaps Jun 1, 2022

vercel bot temporarily deployed to Preview June 1, 2022 07:39 Inactive

weiji14 reviewed Jun 1, 2022

View reviewed changes

fix typos and shorten text based on review

f985703

vercel bot temporarily deployed to Preview June 8, 2022 20:16 Inactive

seisman approved these changes Jun 9, 2022

View reviewed changes

seisman added the final review call This PR requires final review and approval from a second reviewer label Jun 9, 2022

michaelgrund approved these changes Jun 9, 2022

View reviewed changes

weiji14 approved these changes Jun 9, 2022

View reviewed changes

weiji14 merged commit 056483a into GenericMappingTools:main Jun 9, 2022

weiji14 removed the final review call This PR requires final review and approval from a second reviewer label Jun 9, 2022

yvonnefroehlich deleted the improve-examples-categorical-cpt branch June 10, 2022 20:41

yvonnefroehlich mentioned this pull request Jun 14, 2022

Add yvonnefroehlich to AUTHORS.md #1958

Merged

6 tasks

	# Use the minum and maximum of the categorical number code
	# Use the minimum and maximum of the categorical values (alphabetically sorted)

Conversation

yvonnefroehlich commented May 26, 2022

Uh oh!

welcome bot commented May 26, 2022

Uh oh!

yvonnefroehlich commented May 27, 2022

Uh oh!

yvonnefroehlich commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seisman commented May 28, 2022

Uh oh!

Uh oh!

Uh oh!

weiji14 left a comment

Choose a reason for hiding this comment

Uh oh!

weiji14 Jun 1, 2022

Choose a reason for hiding this comment

Uh oh!

yvonnefroehlich Jun 2, 2022

Choose a reason for hiding this comment

Uh oh!

seisman Jun 6, 2022

Choose a reason for hiding this comment

Uh oh!

weiji14 Jun 7, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weiji14 Jun 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yvonnefroehlich Jun 7, 2022

Choose a reason for hiding this comment

Uh oh!

weiji14 Jun 7, 2022

Choose a reason for hiding this comment

Uh oh!

weiji14 Jun 1, 2022

Choose a reason for hiding this comment

Uh oh!

yvonnefroehlich commented Jun 7, 2022

Uh oh!

weiji14 commented Jun 7, 2022

Uh oh!

yvonnefroehlich commented Jun 8, 2022

Uh oh!

michaelgrund commented Jun 8, 2022

Uh oh!

yvonnefroehlich commented Jun 8, 2022

Uh oh!

seisman left a comment

Choose a reason for hiding this comment

Uh oh!

weiji14 left a comment

Choose a reason for hiding this comment

Uh oh!

welcome bot commented Jun 9, 2022

Uh oh!

yvonnefroehlich commented Jun 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yvonnefroehlich commented May 27, 2022 •

edited

Loading

weiji14 Jun 1, 2022 •

edited

Loading