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

Let gmt end better handle -A args #5583

Merged
merged 8 commits into from
Aug 6, 2021
Merged

Let gmt end better handle -A args #5583

merged 8 commits into from
Aug 6, 2021

Conversation

PaulWessel
Copy link
Member

See #5582 for background. This minor change allows the margins and other -A modifiers such as +g, +f, etc.

See #5582 for background.  This minor change allows the margins and other settings such as +g, +f, etc.
@PaulWessel PaulWessel self-assigned this Aug 3, 2021
@PaulWessel PaulWessel changed the title Let gmt end better handle -A args WIP Let gmt end better handle -A args Aug 3, 2021
@PaulWessel
Copy link
Member Author

The key problem is that -A was initially created to do cropping, but then I had lots of other features I wanted added, such as controlling paper color, outlines, margins, scaling, etc, and they all got added to -A. However, none of those modifers are really about cropping, so we added +n to say no crop as well. It should be possible to use +m to add margins to a cropped image and an uncropped image. This is what is failing right now. I wonder if it may be best to let -A with no args turn on cropping (finding the BB) while a -A with arguments does not turn on cropping, and the two can be combined when we want cropping and margin, for example. Wonder if @joa-quim is around to opine on this?

Also fixed errors is the use of GMT_Report.
@PaulWessel
Copy link
Member Author

I have completed the upgrade to psconvert (and gmt end as well) to handle the new options:

  • Crop settings: -A[+r][+u]
  • Scaling/extending settings: -I[+mmargins][+s[m]width[/height]][+Sscale]
  • Color/pen settings: -N[+ffade][+gfill][+i][+p[pen]]

Tests pass and my map that wants to add 0.125i bleed (margin) to the fixed media now works as well. I few notes of things done during the updates:

  1. Parsing of -A anticipates old syntax and sorts modifiers into potential -A, -I, -N sections which are then parsed by the respective parsers (if nonzero args).
  2. The old -I for using ICC profiles has been reborn as -N+i but no-arg -I is also parsed under backwards rules.
  3. A dozen or so GMT_Report calls had been passed Ctrl instead of API as first argument, now fixed.
  4. Modifier order now alphabetical and synopsis and documentations have been updated.
  5. The old -A+n which was added to turn off cropping when -W is used (which automatically turns on -A -P) is still honored under backwards rules but has been replaced by a new +c modifier to -W instead (where it belongs).

The only thing I have not tested is any potential issues when -W is used. Hoping @joa-quim can check for compliance before we remove the WIP.

@joa-quim
Copy link
Member

joa-quim commented Aug 4, 2021

Many changes to eye follow.
-N+g <==> to --PS_PAGE_COLOR, no?

Maybe come out with long-options name for -N & -I

@PaulWessel
Copy link
Member Author

Many changes to eye follow.
-N+g <==> to --PS_PAGE_COLOR, no?

Maybe come out with long-options name for -N & -I

We have had -A+gfill forever and not sure if pS_PAGE_COLOR=rgb works the same way when you extend the region. I will try.

@PaulWessel
Copy link
Member Author

Yes, you can do --PS_PAGE_COLOR or you can do -N+g. I think we added it because it was simpler.

@maxrjones
Copy link
Member

gmt begin options documentation needs updating too, since the new I and N options should be available.

As a GMT user, I would find it helpful to have these deprecations included in the changelog. What do you think of having an additional deprecation section? From a maintenance perspective, it would just mean having another label 'deprecation' and using the 'add-changelog' label for PRs such as this, as well as using descriptive PR titles as per usual. We could adjust the release-drafter workflow to do the rest.

@PaulWessel
Copy link
Member Author

I think that would go useful, too. I suspect most GMT users are resistant to changing how they use GMT and only notice if there is a deprecation if they get a compatibility warning. But they wont get those after we changed the verbosity levels to place compatibility further down the list and beyond the default.

@PaulWessel
Copy link
Member Author

Maybe come out with long-options name for -N & -I

As well as for -A. Perhaps

-A: --crop[+timestamp][+round]
-I: --extend[+margins=args][+size=arg][+scale=arg]
-N: --canvas[+fill=arg][+pen=arg][+icc][+fade=arg]

@maxrjones
Copy link
Member

Not sure if I am missing something from the documentation but I cannot get -N+ffade to do anything:

gmt grdimage @AK_gulf_grav.nc -Jl142W/55N/18/24/1.5c -Bafg > test.ps
gmt psconvert -Tj -A -I+m2c -N+f50 test.ps
open test.jpg

Also, a new notice shows up from gmt end in this PR.

@PaulWessel
Copy link
Member Author

I think it is only meant to be used via movie (fade in fade out). But I will check.

@PaulWessel
Copy link
Member Author

I remove the temporary NOTICE message.

@joa-quim
Copy link
Member

joa-quim commented Aug 5, 2021

As well as for -A.

Sure, but --crop was obvious.

@PaulWessel
Copy link
Member Author

@joa-quim any issues with -W that you can detect? I dont have any good examples to try. As mentioned, the only potential issue would be related to -A+n now being -W+c. You may recall that using -W automatically sets -A -P and we added -A+n to turn -A off, basically, which is an odd way of doing things. The -W+c is better in that sense. But not tested.

@joa-quim
Copy link
Member

joa-quim commented Aug 5, 2021

Try the Patagonia example in the psconvert man page.
Some Windows update screw the compiler and can't currently build.

@PaulWessel PaulWessel changed the title WIP Let gmt end better handle -A args Let gmt end better handle -A args Aug 5, 2021
@PaulWessel
Copy link
Member Author

Will do.

@PaulWessel
Copy link
Member Author

Patagonia example worked, I could import that geotiff in Google Earth Pro and it obliquely laid down and fit the background. So I will merged shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add-changelog Add PR to the changelog deprecation Deprecating a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants