-
Notifications
You must be signed in to change notification settings - Fork 217
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
Figure: Remove deprecated "timestamp" ("U") parameter, use "Figure.timestamp" instead (deprecated since v0.9.0) #3045
Conversation
@@ -567,17 +567,6 @@ def new_module(*args, **kwargs): | |||
) | |||
warnings.warn(msg, category=SyntaxWarning, stacklevel=2) | |||
|
|||
# timestamp (U) is deprecated since v0.9.0. |
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.
After this PR, the expected behaviors are:
- if
timestamp=True
is used, Python will report the parameter is not recognized - if single-letter parameter
U=True
is used, PyGMT should tell users thatU
is not supported andFigure.timestamp
should be used.
After removing lines 570-580, parameter timestamp
is no longer recognized, but single-letter parameter U
will still be processed. Thus, instead of removing these lines completely, we should change them to something like:
# timestamp (U)is deprecated since v0.9.0.
if "U" in kwargs:
msg = (
"Parameter timestamp/U is no longer supported since v0.12.0."
"Use Figure.timestamp() instead."
)
raise GMTInvalidInput(msg)
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.
pygmt/tests/test_timestamp.py
Outdated
@pytest.mark.mpl_image_compare(filename="test_timestamp.png") | ||
def test_timestamp_deprecated_u(faketime): | ||
""" | ||
Check if the deprecated parameter 'U' works but raises a warning. | ||
""" | ||
fig = Figure() | ||
with pytest.warns(expected_warning=SyntaxWarning) as record: | ||
with config(FORMAT_TIME_STAMP=faketime): | ||
# plot nothing (the data is outside the region) but a timestamp | ||
fig.plot(x=0, y=0, style="p", projection="X1c", region=[1, 2, 1, 2], U=True) | ||
assert len(record) == 1 # check that only one warning was raised | ||
return fig |
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.
Need to modify this test to check that the GMTInvalidInput exception is raised if U
is used.
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.
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.
/format |
Description of proposed changes
This PR removes the deprecated
timestampe
(U
) parameter from all plotting methods (deprecated since v0.9.0).The standalone method
Figure.timestamp
should be used instead.Fixes partly #3033
Related to #2135
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash command is:
/format
: automatically format and lint the code