-
Notifications
You must be signed in to change notification settings - Fork 235
AliasSystem: Support adding a suffix to a value and simplify Figure.wiggle #4259
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
Changes from all commits
5d81d2a
9c4dde4
a8f9851
2b94c3c
fdf2c1a
51dee3c
1647476
f380b6d
7c81056
12e451c
7ceea09
b920d4a
51c4dc2
25f47aa
cc28eff
acb6e36
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,33 +13,6 @@ | |
| from pygmt.src._common import _parse_position | ||
|
|
||
|
|
||
| def _parse_fills(positive_fill, negative_fill): | ||
| """ | ||
| Parse the positive_fill and negative_fill parameters. | ||
|
|
||
| >>> _parse_fills("red", "blue") | ||
| ['red+p', 'blue+n'] | ||
| >>> _parse_fills(None, "blue") | ||
| 'blue+n' | ||
| >>> _parse_fills("red", None) | ||
| 'red+p' | ||
| >>> _parse_fills(None, None) | ||
| """ | ||
| _fills = [] | ||
| if positive_fill is not None: | ||
| _fills.append(positive_fill + "+p") | ||
| if negative_fill is not None: | ||
| _fills.append(negative_fill + "+n") | ||
|
|
||
| match len(_fills): | ||
| case 0: | ||
| return None | ||
| case 1: | ||
| return _fills[0] | ||
| case 2: | ||
| return _fills | ||
|
|
||
|
|
||
| @fmt_docstring | ||
| # TODO(PyGMT>=0.20.0): Remove the deprecated 'fillpositive' parameter. | ||
| # TODO(PyGMT>=0.20.0): Remove the deprecated 'fillnegative' parameter. | ||
|
|
@@ -71,8 +44,8 @@ def wiggle( # noqa: PLR0913 | |
| length: float | str | None = None, | ||
| label: str | None = None, | ||
| label_alignment: Literal["left", "right"] | None = None, | ||
| positive_fill=None, | ||
| negative_fill=None, | ||
| positive_fill: str | None = None, | ||
| negative_fill: str | None = None, | ||
| projection: str | None = None, | ||
| region: Sequence[float | str] | str | None = None, | ||
| frame: str | Sequence[str] | bool = False, | ||
|
|
@@ -115,7 +88,6 @@ def wiggle( # noqa: PLR0913 | |
| $table_classes. | ||
| Use parameter ``incols`` to choose which columns are x, y, z, | ||
| respectively. | ||
|
|
||
| position | ||
| Position of the vertical scale on the plot. It can be specified in multiple | ||
| ways: | ||
|
|
@@ -141,9 +113,9 @@ def wiggle( # noqa: PLR0913 | |
| or **p** to indicate the distance unit (centimeters, inches, or | ||
| points); if no unit is given we use the default unit that is | ||
| controlled by :gmt-term:`PROJ_LENGTH_UNIT`. | ||
| positive_fill : str | ||
| positive_fill | ||
| Set color or pattern for filling positive wiggles [Default is no fill]. | ||
| negative_fill : str | ||
| negative_fill | ||
| Set color or pattern for filling negative wiggles [Default is no fill]. | ||
| track : str | ||
| Draw track [Default is no track]. Append pen attributes to use | ||
|
|
@@ -174,8 +146,6 @@ def wiggle( # noqa: PLR0913 | |
| kwdict={"length": length, "label": label, "label_alignment": label_alignment}, | ||
| ) | ||
|
|
||
| _fills = _parse_fills(positive_fill, negative_fill) | ||
|
|
||
| aliasdict = AliasSystem( | ||
| D=[ | ||
| Alias(position, name="position"), | ||
|
|
@@ -188,7 +158,10 @@ def wiggle( # noqa: PLR0913 | |
| ), | ||
| Alias(label, name="label", prefix="+l"), | ||
| ], | ||
| G=Alias(_fills, name="positive_fill/negative_fill"), | ||
| G=[ | ||
| Alias(positive_fill, name="positive_fill", suffix="+p"), | ||
| Alias(negative_fill, name="negative_fill", suffix="+n"), | ||
|
Comment on lines
+162
to
+163
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe take this opportunity to update the type-hint of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 12e451c
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should remove |
||
| ], | ||
| ).add_common( | ||
| B=frame, | ||
| J=projection, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be confusing if we allowed both
prefixandsuffixwhen value is True? E.g. at #4234 (comment), we usedprefix, but it could also besuffix.Or I guess it doesn't matter, and we should just be consistent that every if-branch in this
_to_stringfunction handles prefix and suffix.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefixandsuffixcannot coexist. This function does do the check to avoid too much function overhead. It's our responsibility to pass the correctprefix/suffixvalues.