Skip to content

Correct column naming and width unit in ternary example #7135

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

Merged
merged 3 commits into from
Dec 8, 2022

Conversation

michaelgrund
Copy link
Member

Description of proposed changes

Based on #2227 this PR corrects the column naming in the ternary example.

Reminders

  • Make sure that your code follows our style. Use the other functions/files as a basis.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Describe changes to function behavior and arguments in a comment below the function declaration.
  • If adding new functionality, add a detailed description to the documentation and/or an example.

@yvonnefroehlich
Copy link
Member

yvonnefroehlich commented Dec 6, 2022

Also the example for psternary has to be corrected, please.

Maybe correcting the test (see https://github.com/GenericMappingTools/gmt/blob/master/test/psternary/ternary.sh) makes also sense? (Please note the negative sign in -JX-6i.)

@joa-quim
Copy link
Member

joa-quim commented Dec 6, 2022

Note that ternary.sh test produces the same output as the one shown in the Matlab central example. What seems to be lacking, ans a Yvonne noticed, is the -JX-6i in the example of psternary. And while we are at it, that example should be using cm and not inches.

@yvonnefroehlich
Copy link
Member

Note that ternary.sh test produces the same output as the one shown in the Matlab central example.

I do not get the same figures. Please note the axis directions highlighted by the purple ellipses.
Test psternary output:
gmt_psternary_test_marked

MATLAB file exchange output:
teneray_matlab_fileexchange_marked

@yvonnefroehlich
Copy link
Member

yvonnefroehlich commented Dec 6, 2022

What seems to be lacking, ans a Yvonne noticed, is the -JX-6i in the example of psternary.

Yes, without the minus sign in -JX-6i the plots are "rotated" to each other. But at least, after the renaming by @michaelgrund, both appear for me identical and correct regarding annotations and labels - please correct me if I am wrong and overlooked something...
teneray (modern mode) after renaming output:
gmt_ternary_modern_corr_marked02

MATLAB file exchange output:
teneray_matlab_fileexchange_marked02

@joa-quim
Copy link
Member

joa-quim commented Dec 6, 2022

The Matlab fig in FEX does not show any labels at the triangle vertices so that can't be used for comparison, and the labels at the axes are shorter (Water instead of "Water component"). Otherwise it's equal to what is produced by the ternary.sh test that uses -JX-6i. So, all that needs to be changed in the example is that

ternary

@michaelgrund
Copy link
Member Author

I agree with @yvonnefroehlich that after the renaming and changing -JX-6i to -JX6i all figures (from FEX, ternary example and fig from the test) are identical, with the exception that the whole diagram is rotated. The difference between @joa-quim s last figure and the one from FEX is that the values (0 - 100% or 0-1) are in opposite directions.

So, to conclude what needs to be changed:

  1. renaming (as already done in this PR for the example) for all appearances
  2. change -JX-6i to -JX6i in ternary.sh

@yvonnefroehlich
Copy link
Member

yvonnefroehlich commented Dec 7, 2022

I agree with @yvonnefroehlich that after the renaming and changing -JX-6i to -JX6i all figures (from FEX, ternary example and fig from the test) are identical, with the exception that the whole diagram is rotated. The difference between @joa-quim s last figure and the one from FEX is that the values (0 - 100% or 0-1) are in opposite directions.

The second aspect is what I meant with "Please note the axis directions" and tried to highlight with the purple ellipses in #7135 (comment) 🙂.

So, to conclude what needs to be changed:

  1. renaming (as already done in this PR for the example) for all appearances
  2. change -JX-6i to -JX6i in ternary.sh

Regarding the second point, I feel it makes sense to addionally change the unit from inches to centimeters as mention by @joa-quim in #7135 (comment).

Hm. Probably adding a colorbar also to the examples in the docs (in the test there is already a colorbar) would make the figure better interpretable?

@joa-quim
Copy link
Member

joa-quim commented Dec 7, 2022

OK, you are right that the axes have opposing senses in the GMT and FEX when -JX- is used. But we will hardly produce the exact same map with GMT and Matlab because in Matlab the variable 1 is on the left side of the triangle whilst in GMT it's on the base.
On the other side the ternary.sh is a test and not a figure exposed to public. Changing it will require updating the PS file without bringing anything else to the tests results.

In summary, I think it's correct to change the column naming but no need to change anything else (except the damn inches)

@PaulWessel
Copy link
Member

I agree changing the test script only adds more bellyache with PS update. But go ahead and update the other things.

@michaelgrund michaelgrund changed the title Correct column naming in ternary example Correct column naming and width unit in ternary example Dec 7, 2022
@seisman seisman merged commit 6ecce0b into GenericMappingTools:master Dec 8, 2022
@michaelgrund michaelgrund deleted the patch-4 branch September 3, 2024 13:07
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