-
Notifications
You must be signed in to change notification settings - Fork 235
AliasSystem: Migrate the 'registration' parameter to the new alias system and support descriptive arguments #4182
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
Conversation
weiji14
left a comment
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.
thinking if we should support the
GridRegistrationenums
Ideally yes, but probably do in another PR. Not sure how easy it is to convert an IntEnum to support StrEnum also (maybe ReprEnum?).
| ] = "number", | ||
| quantile_value: float = 50, | ||
| region: Sequence[float | str] | str | None = None, | ||
| registration: Literal["gridline", "pixel"] | bool = False, |
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.
Docstring for registration should be updated to 1) add bool type hint, and 2) indicate that registration=True means pixel registration.
pygmt/pygmt/helpers/decorators.py
Lines 279 to 283 in 25f2656
| "registration": r""" | |
| registration : str | |
| **g**\|\ **p**. | |
| Force gridline (**g**) or pixel (**p**) node registration | |
| [Default is **g**\ (ridline)].""", |
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.
add bool type hint,
Type hints are now added via function signatures, not in docstrings.
GMT documentation: https://docs.generic-mapping-tools.org/6.6/gmt.html#nodereg-full
The GMT CLI syntax is
-r[g|p]. Here, we use the descriptive values "gridline" and "pixel".I'm still thinking if we should support the
GridRegistrationenums (https://www.pygmt.org/dev/api/generated/pygmt.enums.GridRegistration.html#pygmt.enums.GridRegistration), i.e.,@GenericMappingTools/pygmt-maintainers What do you think?