-
Notifications
You must be signed in to change notification settings - Fork 245
Figure: Remove deprecated "timestamp" ("U") parameter, use "Figure.timestamp" instead (deprecated since v0.9.0) #3045
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 10 commits
af5f860
cca6dba
d00b48c
d7f1a15
6977a57
1501ccc
42fab3b
554231c
b45b45a
51e8659
f8a8f4e
6154526
a02315e
ed766bb
b20ebb2
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 |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
| Test Figure.timestamp. | ||
| """ | ||
| import pytest | ||
| from pygmt import Figure, config | ||
| from pygmt import Figure | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module", name="faketime") | ||
|
|
@@ -93,38 +93,3 @@ def test_timestamp_text_truncated(): | |
| fig.timestamp(text="0123456789" * 7) | ||
| assert len(record) == 1 # check that only one warning was raised | ||
| return fig | ||
|
seisman marked this conversation as resolved.
|
||
|
|
||
|
|
||
| @pytest.mark.mpl_image_compare(filename="test_timestamp.png") | ||
| def test_timestamp_deprecated_timestamp(faketime): | ||
| """ | ||
| Check if the deprecated parameter 'timestamp' 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], | ||
| timestamp=True, | ||
| ) | ||
| assert len(record) == 1 # check that only one warning was raised | ||
| return fig | ||
|
|
||
|
|
||
| @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 | ||
|
Comment on lines
-119
to
-130
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. Need to modify this test to check that the GMTInvalidInput exception is raised if
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.
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. |
||
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:
timestamp=Trueis used, Python will report the parameter is not recognizedU=Trueis used, PyGMT should tell users thatUis not supported andFigure.timestampshould be used.After removing lines 570-580, parameter
timestampis no longer recognized, but single-letter parameterUwill still be processed. Thus, instead of removing these lines completely, we should change them to something like: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.
Ah, OK. Thanks! I have tried to change this in the commits 6977a57, 1501ccc, and 42fab3b. I also changed this in PR #3044 for
xshift(X) andyshift(Y).