-
Notifications
You must be signed in to change notification settings - Fork 224
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
Wrap inset #788
Wrap inset #788
Conversation
Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Dongdong Tian <[email protected]>
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.
Fingers crossed that we can finalize this before the PyGMT meeting!
The PR looks good to me after addressing the above minor comments. |
Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Dongdong Tian <[email protected]>
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.
Phew, thanks for your patience @willschlitzer. Feel free to merge once the tests pass! 🚀
*Wrap inset function to use a context manager *Create src/inset.py and import inset function into base_plotting.py Co-authored-by: Wei Ji <[email protected]> Co-authored-by: Dongdong Tian <[email protected]>
Description of proposed changes
As discussed in #663, this is wrapping the GMT function
inset
. After some back and forth (and an attempt at Option 2), this PR goes with Option 1 to use a context manager andwith
statement to callinset
.Fixes #663
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Notes
/format
in the first line of a comment to lint the code automatically