-
Notifications
You must be signed in to change notification settings - Fork 228
New Alias System: Add the private _to_string function #3986
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
base: main
Are you sure you want to change the base?
Conversation
01b8267
to
8f5c52b
Compare
pygmt/alias.py
Outdated
match mapping: | ||
case False: | ||
pass | ||
case True: | ||
value = value[0] | ||
case Mapping(): | ||
if value not in mapping and value not in mapping.values(): | ||
raise GMTValueError( | ||
value, | ||
description="value for parameter {name!r}" if name else "value", | ||
choices=mapping.keys(), | ||
) |
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.
mapping=True
means using the first letter of a long-form argument as its short-form argument. For example, resolution="high"
will be mapped to resolution="h"
. If we don't support mapping=True
, lines 112-123 can be simplified to a few lines:
if value not in mapping and value not in mapping.values():
raise GMTValueError(
value,
description="value for parameter {name!r}" if name else "value",
choices=mapping.keys(),
)
but without mapping=True
, we need to write something like
mapping={"full": "f", "high": "h", "intermediate": "i", "low": "l", "crude": "c"}
when calling the _to_string
function.
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.
I think better to be explicit (use the full mapping {"key": "value"} dict), and just have one way to parse things. We can use the first letter for resolution
, but for others like interpolation
/-n
, where the mapping is {"b-spline": "b", "bicubic": "c", "bilinear": "l", ...}
, it doesn't work.
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.
I've removed mapping=True
in commit c499aab.
size: int | Sequence[int] | None = None, | ||
ndim: int = 1, | ||
name: str | None = None, |
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.
Missing docstring for these three parameters?
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.
Added in 232ff52.
No description provided.