Skip to content

Figure.grdimage: Remove the unsupported 'img_out'/'A' parameter - #2907

Merged
seisman merged 3 commits into
mainfrom
grdimage/remove-imgout
Dec 25, 2023
Merged

Figure.grdimage: Remove the unsupported 'img_out'/'A' parameter#2907
seisman merged 3 commits into
mainfrom
grdimage/remove-imgout

Conversation

@seisman

@seismanseisman commented Dec 21, 2023

Copy link
Copy Markdown
Member

As proposed in #2765 (comment), we will NOT support the img_out/A parameter in the Figure.grdimage method. The related feature will be added to a new function/method if requested.

Supersede #2765.

@seismanseisman added the deprecation Deprecating a feature label Dec 21, 2023
@seismanseisman added this to the 0.11.0 milestone Dec 21, 2023
@seisman
seismanforce-pushed the grdimage/remove-imgout branch from 7923939 to 907d658CompareDecember 21, 2023 10:47
@seismanseisman added the needs review This PR has higher priority and needs review. label Dec 21, 2023
@seisman
seisman requested a review from a teamDecember 22, 2023 14:23

@weiji14weiji14 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor suggestions, otherwise ok.

Comment threadpygmt/tests/test_grdimage.py Outdated
Comment threadpygmt/src/grdimage.py
kwargs = self._preprocess(**kwargs)

# Do not support -A option
if any(kwargs.get(arg) is not None for arg in ["A", "img_out"]):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ifany(kwargs.get(arg) isnotNoneforargin ["A", "img_out"]):
ifany(kwargs.get(arg) forargin ["A", "img_out"]):

Should be ok to remove is not None, e.g.:

kwargs= {"img_out": "temp.nc"}
ifany(kwargs.get(arg) forargin ["A", "img_out"]):
raiseException

should raise an Exception.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't raise an exception if an empty string is given:

kwargs = {"img_out": ""}
if any(kwargs.get(arg) for arg in ["A", "img_out"]):
raise Exception

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, keep it as is then.

seismanand others added 2 commits December 25, 2023 15:58
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
@seismanseisman added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Dec 25, 2023
@codspeed-hq

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #2907 will not alter performance

Comparing grdimage/remove-imgout (aa85ac6) with main (013014b)

Summary

✅ 1 untouched benchmarks

@seismanseisman removed the final review call This PR requires final review and approval from a second reviewer label Dec 25, 2023
@seisman
seisman merged commit ff3dbb4 into mainDec 25, 2023
@seisman
seisman deleted the grdimage/remove-imgout branch December 25, 2023 08:17
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecationDeprecating a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@seisman@weiji14