Skip to content
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

Copy method for declarative interface #1884

Merged

Conversation

23ccozad
Copy link
Contributor

@23ccozad 23ccozad commented May 24, 2021

Description Of Changes

Adds a .copy() method to just about every class in declarative.py. This includes Plots2D (and hence all of its subclasses), MapPanel, PanelContainer, and PlotObs. The only class excluded is Panel, since it is currently empty.

This obj.copy() is equivalent to using import copy and copy.copy(obj), except when obj is a MapPanel.

The .copy() method for MapPanel required some additional changes in order to get it working. @dopplershift and I found that we needed to set allow_none=True for the parent trait of MapPanel in order to accommodate the process of copying a MapPanel that may not have been assigned a parent yet. A __copy__() method was written explicitly for MapPanel since a deeper copy of MapPanel's plots trait is needed in order for the copy to work correctly.

Test function test_copy() was added in test_declarative.py

Checklist

@CLAassistant
Copy link

CLAassistant commented May 24, 2021

CLA assistant check
All committers have signed the CLA.

@23ccozad 23ccozad marked this pull request as ready for review May 25, 2021 15:34
Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Overall, this looks great. Just a few minor clean-ups. There is the question of whether we can do more in testing that our copies are actually copying some trait values across

src/metpy/plots/declarative.py Outdated Show resolved Hide resolved
tests/plots/test_declarative.py Outdated Show resolved Hide resolved
tests/plots/test_declarative.py Outdated Show resolved Hide resolved
tests/plots/test_declarative.py Show resolved Hide resolved
src/metpy/plots/declarative.py Outdated Show resolved Hide resolved
@dopplershift dopplershift added this to the 1.1.0 milestone May 25, 2021
@dopplershift dopplershift added Area: Plots Pertains to producing plots Type: Enhancement Enhancement to existing functionality labels May 25, 2021
@23ccozad 23ccozad force-pushed the copy_method_declarative_interface branch from d4363d5 to 0eed012 Compare May 25, 2021 21:20
MapPanel needed an explicity written __copy__() to make sure the copy process worked correctly. All other classes in declarative.py did not need it. A .copy() method is also provided for ease of use.
@23ccozad 23ccozad force-pushed the copy_method_declarative_interface branch from 0eed012 to 8d0b963 Compare May 25, 2021 21:31
@dopplershift dopplershift merged commit 23c0141 into Unidata:main May 26, 2021
@23ccozad 23ccozad deleted the copy_method_declarative_interface branch June 1, 2021 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Plots Pertains to producing plots Type: Enhancement Enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

copy() method for declarative interface
3 participants