Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
078f382
remove __getstate__ in abstract_data_source.py
aaronayres35 Apr 7, 2021
3745495
remove __getstate__ from axis.py
aaronayres35 Apr 7, 2021
b7f7210
remove __getstate__ from grid.py
aaronayres35 Apr 7, 2021
72700bb
remove __getstate__ from base_xy_plot.py
aaronayres35 Apr 7, 2021
77c0e59
remove __getstate__ from cross_plot_frame.py, also delete commented o…
aaronayres35 Apr 7, 2021
80a16b9
remove __getstate__ from lineplot.py
aaronayres35 Apr 7, 2021
fae52fa
remove __getstate__ from multi_line_plot.py
aaronayres35 Apr 7, 2021
c7c93ab
remove __getstate__ from tools/simple_zoom.py
aaronayres35 Apr 7, 2021
f38cbcc
remove __getstate__ from simple_plot_frame.py
aaronayres35 Apr 7, 2021
eeea965
remove __getstate__ from abstract_mapper.py (_cache_valid is not a tr…
aaronayres35 Apr 7, 2021
6fc674f
fix array_data_source.py __getstate__ and its tests
aaronayres35 Apr 7, 2021
ba66fc8
update plot_containers.py
aaronayres35 Apr 7, 2021
2152d9b
delete commented out test skip
aaronayres35 Apr 7, 2021
a984b40
delete _pickles commented out code
aaronayres35 Apr 7, 2021
6bfb912
make all private cache related traits transient (might be overrkill)
aaronayres35 Apr 13, 2021
c005b55
Merge branch 'master' into audit-dunder-state-methods
aaronayres35 Apr 13, 2021
fd4b2c9
Merge branch 'master' into audit-dunder-state-methods
aaronayres35 Apr 14, 2021
55893e0
fix merge conflict issues
aaronayres35 Apr 15, 2021
f3395f0
bring back #:
aaronayres35 Apr 15, 2021
14789fd
Merge branch 'master' into audit-dunder-state-methods
aaronayres35 Apr 15, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions chaco/abstract_data_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ class AbstractDataSource(HasTraits):
#: The dimensionality of the value at each index point.
#: Subclasses re-declare this trait as a read-only trait with
#: the right default value.
value_dimension = DimensionTrait
value_dimension = DimensionTrait(transient=True)

#: The dimensionality of the indices into this data source.
#: Subclasses re-declare this trait as a read-only trait with
#: the right default value.
index_dimension = DimensionTrait
index_dimension = DimensionTrait(transient=True)

#: A dictionary keyed on strings. In general, it maps to indices (or tuples
#: of indices, depending on **value_dimension**), as in the case of
Expand All @@ -51,7 +51,7 @@ class AbstractDataSource(HasTraits):

#: Should the data that this datasource refers to be serialized when
#: the datasource is serialized?
persist_data = Bool(True)
persist_data = Bool(True, transient=True)

# ------------------------------------------------------------------------
# Abstract methods
Expand Down Expand Up @@ -118,13 +118,3 @@ def get_bounds(self):

def _metadata_default(self):
return {"selections": [], "annotations": []}

def __getstate__(self):
state = super(AbstractDataSource, self).__getstate__()

# everything but 'metadata'
for key in ["value_dimension", "index_dimension", "persist_data"]:
if key in state:
del state[key]

return state
7 changes: 0 additions & 7 deletions chaco/abstract_mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,6 @@ def map_data_array(self, screen_vals):
# ------------------------------------------------------------------------
# Persistence-related methods
# ------------------------------------------------------------------------
def __getstate__(self):
state = super(AbstractMapper, self).__getstate__()
for key in ["_cache_valid"]:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

_cache_valid is not a trait on this class or any of its base classes

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri Apr 13, 2021

Choose a reason for hiding this comment

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

sigh. but it is a trait on the subclasses e.g. Base1DMapper so those need to be updated

Copy link
Copy Markdown
Contributor Author

@aaronayres35 aaronayres35 Apr 13, 2021

Choose a reason for hiding this comment

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

Ah you are right. TBH I believe that all related traits should be transient even if they weren't perviously involved in a __getstate__method. For example other classes have _screen_cache_valid or _selection_cache_valid traits. I think I will update all of these (to be specific I am going to update any privately named cache trait) to be transient as I can't see why they shouldn't all be so

It also doesn't make sense that for some objects the _cache_valid and similar traits would be transient traits but on others not

if key in state:
del state[key]

return state

def _post_load(self):
self._cache_valid = False
Expand Down
2 changes: 1 addition & 1 deletion chaco/array_data_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ def _metadata_items_changed(self, event):
# ------------------------------------------------------------------------

def __getstate__(self):
state = self.__dict__.copy()
state = super(ArrayDataSource, self).__getstate__()
if not self.persist_data:
state.pop("_data", None)
state.pop("_cached_mask", None)
Expand Down
64 changes: 18 additions & 46 deletions chaco/axis.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,24 +177,24 @@ class PlotAxis(AbstractOverlay):

# Cached position calculations

_tick_list = List # These are caches of their respective positions
_tick_positions = ArrayOrNone()
_tick_label_list = ArrayOrNone()
_tick_label_positions = ArrayOrNone()
_tick_label_bounding_boxes = List
_major_axis_size = Float
_minor_axis_size = Float
_major_axis = Array
_title_orientation = Array
_title_angle = Float
_origin_point = Array
_inside_vector = Array
_axis_vector = Array
_axis_pixel_vector = Array
_end_axis_point = Array

ticklabel_cache = List
_cache_valid = Bool(False)
_tick_list = List(transient=True) # These are caches of their respective positions
_tick_positions = ArrayOrNone(transient=True)
_tick_label_list = ArrayOrNone(transient=True)
_tick_label_positions = ArrayOrNone(transient=True)
_tick_label_bounding_boxes = List(transient=True)
_major_axis_size = Float(transient=True)
_minor_axis_size = Float(transient=True)
_major_axis = Array(transient=True)
_title_orientation = Array(transient=True)
_title_angle = Float(transient=True)
_origin_point = Array(transient=True)
_inside_vector = Array(transient=True)
_axis_vector = Array(transient=True)
_axis_pixel_vector = Array(transient=True)
_end_axis_point = Array(transient=True)

ticklabel_cache = List(transient=True)
_cache_valid = Bool(False, transient=True)

# ------------------------------------------------------------------------
# Public methods
Expand Down Expand Up @@ -831,34 +831,6 @@ def _title_angle_default(self):
# Persistence-related methods
# ------------------------------------------------------------------------

def __getstate__(self):
dont_pickle = [
"_tick_list",
"_tick_positions",
"_tick_label_list",
"_tick_label_positions",
"_tick_label_bounding_boxes",
"_major_axis_size",
"_minor_axis_size",
"_major_axis",
"_title_orientation",
"_title_angle",
"_origin_point",
"_inside_vector",
"_axis_vector",
"_axis_pixel_vector",
"_end_axis_point",
"_ticklabel_cache",
"_cache_valid",
]

state = super(PlotAxis, self).__getstate__()
for key in dont_pickle:
if key in state:
del state[key]

return state

def __setstate__(self, state):
super(PlotAxis, self).__setstate__(state)
self._mapper_changed(None, self.mapper)
Expand Down
2 changes: 1 addition & 1 deletion chaco/base_1d_mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class Base1DMapper(AbstractMapper):

# If the subclass uses a cache, _cache_valid is maintained to
# monitor its status
_cache_valid = Bool(False)
_cache_valid = Bool(False, transient=True)

# Indicates whether or not the bounds have been set at all, or if they
# are at their initial default values.
Expand Down
12 changes: 6 additions & 6 deletions chaco/base_1d_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,22 +64,22 @@ class Base1DPlot(AbstractPlotRenderer):
# ------------------------------------------------------------------------

#: flag whether the data cache is valid
_cache_valid = Bool(False)
_cache_valid = Bool(False, transient=True)

#: cache of the index values in data space
_cached_data = Any()
_cached_data = Any(transient=True)

#: cache of the sorted index values in data space
_cached_data_pts_sorted = Any()
_cached_data_pts_sorted = Any(transient=True)

#: cache of the sorted indices of the index values
_cached_data_argsort = Any()
_cached_data_argsort = Any(transient=True)

#: flag whether the screen coordinates are valid
_screen_cache_valid = Bool(False)
_screen_cache_valid = Bool(False, transient=True)

#: cache holding the screen coordinates of the index values
_cached_screen_pts = Any()
_cached_screen_pts = Any(transient=True)

# ------------------------------------------------------------------------
# AbstractPlotRenderer interface
Expand Down
4 changes: 2 additions & 2 deletions chaco/base_contour_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ class BaseContourPlot(Base2DPlot):
# ------------------------------------------------------------------------

# Is the cached level data valid?
_level_cache_valid = Bool(False)
_level_cache_valid = Bool(False, transient=True)

# Is the cached color data valid?
_colors_cache_valid = Bool(False)
_colors_cache_valid = Bool(False, transient=True)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the cache valid flags arent transient, then the caches themselves should also be transient - _levels and _colors.

I would think HasTraits would ignore traits that are private i.e. that start an _. If that isn't the case, there are a lot more private traits that we will want to mark as transient.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As you suggested, I printed state inside the ArrayDataSource.__getstate__ method on master and ran the test suite. I do see _ prefixed traits, eg _data, '_min_index', etc. So it looks like these need to be addressed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If that isn't the case, there are a lot more private traits that we will want to mark as transient.

Does this mean all private traits be transient?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As you suggested, I printed state inside the ArrayDataSource.getstate method on master and ran the test suite. I do see _ prefixed traits, eg _data, '_min_index', etc. So it looks like these need to be addressed

Sigh. I was afraid of that.

Does this mean all private traits be transient?

Let's not do that. Definitely not in this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, I imagine the code will recreate the cache if a *cache* is False, which it is by default because we're marking it as a transient. So, I think we can ignore all of the traits that are recalculated based on the *cache* flag. - i.e. nothing actionable in this PR.


# List of levels and their associated line properties.
_levels = List
Expand Down
1 change: 0 additions & 1 deletion chaco/base_plot_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ def __setattr__(self, name, value):
super(BasePlotFrame, self).__setattr__(name, value)

### Persistence ###########################################################
# _pickles = ("_frame_slots", "_components", "fit_components", "fit_window")

def post_load(self, path=None):
super(BasePlotFrame, self).post_load(path)
Expand Down
21 changes: 4 additions & 17 deletions chaco/base_xy_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,18 +121,18 @@ class BaseXYPlot(AbstractPlotRenderer):
# ------------------------------------------------------------------------

# Are the cache traits valid? If False, new ones need to be compute.
_cache_valid = Bool(False)
_cache_valid = Bool(False, transient=True)

# Cached array of (x,y) data-space points; regardless of self.orientation,
# these points are always stored as (index_pt, value_pt).
_cached_data_pts = Array
_cached_data_pts = Array(transient=True)

# Cached array of (x,y) screen-space points.
_cached_screen_pts = Array
_cached_screen_pts = Array(transient=True)

# Does **_cached_screen_pts** contain the screen-space coordinates
# of the points currently in **_cached_data_pts**?
_screen_cache_valid = Bool(False)
_screen_cache_valid = Bool(False, transient=True)

# Reference to a spatial subdivision acceleration structure.
_subdivision = Any
Expand Down Expand Up @@ -721,19 +721,6 @@ def _use_subdivision_changed(self, old, new):
# Persistence
# ------------------------------------------------------------------------

def __getstate__(self):
state = super(BaseXYPlot, self).__getstate__()
for key in [
"_cache_valid",
"_cached_data_pts",
"_screen_cache_valid",
"_cached_screen_pts",
]:
if key in state:
del state[key]

return state

def __setstate__(self, state):
super(BaseXYPlot, self).__setstate__(state)
if self.index is not None:
Expand Down
4 changes: 2 additions & 2 deletions chaco/cmap_image_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ class CMapImagePlot(ImagePlot):
# ------------------------------------------------------------------------

# Is the mapped image valid?
_mapped_image_cache_valid = Bool(False)
_mapped_image_cache_valid = Bool(False, transient=True)

# Cache of the fully mapped RGB(A) image.
_cached_mapped_image = Any
_cached_mapped_image = Any(transient=True)

# ------------------------------------------------------------------------
# Public methods
Expand Down
8 changes: 4 additions & 4 deletions chaco/contour_line_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@ class ContourLinePlot(BaseContourPlot):
# ------------------------------------------------------------------------

# Are the cached contours valid? If False, new ones need to be computed.
_contour_cache_valid = Bool(False)
_contour_cache_valid = Bool(False, transient=True)

# Cached collection of traces.
_cached_contours = Dict
_cached_contours = Dict(transient=True)

# Is the cached width data valid?
_widths_cache_valid = Bool(False)
_widths_cache_valid = Bool(False, transient=True)

# Is the cached style data valid?
_styles_cache_valid = Bool(False)
_styles_cache_valid = Bool(False, transient=True)

# Cached list of line widths
_widths = List
Comment on lines 60 to 61
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like this is technically a cache but like I said earlier, this will be ignored/recomputed because the _widths_cache_valid trait is False - no action required, just pointing it out.

Expand Down
4 changes: 2 additions & 2 deletions chaco/contour_poly_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ class ContourPolyPlot(BaseContourPlot):
# ------------------------------------------------------------------------

# Are the cached contours valid? If False, new ones need to be computed.
_poly_cache_valid = Bool(False)
_poly_cache_valid = Bool(False, transient=True)

# Cached collection of traces.
_cached_polys = Dict
_cached_polys = Dict(transient=True)

# ------------------------------------------------------------------------
# Private methods
Expand Down
14 changes: 1 addition & 13 deletions chaco/cross_plot_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class CrossPlotFrame(BasePlotFrame):
bottom_height = Float(50.0)

# Does the component need to do a layout call?
_layout_needed = Bool(True)
_layout_needed = Bool(True, transient=True)

def __init__(self, **kwtraits):
bounds = kwtraits.pop("bounds", list(self.default_bounds))
Expand Down Expand Up @@ -152,15 +152,3 @@ def _do_layout(self):
if "v" not in slot.resizable:
slot.outer_height = preferred_size[1]
slot.do_layout()

### Persistence ###########################################################

# _pickles = ("left_width", "right_width", "top_height", "bottom_height")

def __getstate__(self):
state = super(CrossPlotFrame, self).__getstate__()
for key in ["_layout_needed"]:
if key in state:
del state[key]

return state
21 changes: 4 additions & 17 deletions chaco/grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,13 @@ class PlotGrid(AbstractOverlay):
# Private traits; mostly cached information
# ------------------------------------------------------------------------

_cache_valid = Bool(False)
_tick_list = Any
_tick_positions = Any
_cache_valid = Bool(False, transient=True)
_tick_list = Any(transient=True)
_tick_positions = Any(transient=True)

# An array (N,2) of start,end positions in the transverse direction
# i.e. the direction corresponding to self.orientation
_tick_extents = Any
_tick_extents = Any(transient=True)

# _length = Float(0.0)

Expand Down Expand Up @@ -444,19 +444,6 @@ def _orientation_changed(self):

### Persistence ###########################################################

def __getstate__(self):
state = super(PlotGrid, self).__getstate__()
for key in [
"_cache_valid",
"_tick_list",
"_tick_positions",
"_tick_extents",
]:
if key in state:
del state[key]

return state

def _post_load(self):
super(PlotGrid, self)._post_load()
self._mapper_changed(None, self.mapper)
Expand Down
4 changes: 2 additions & 2 deletions chaco/image_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ class ImageData(AbstractDataSource):
_data = ImageTrait

# Is the bounds cache valid? If False, it needs to be computed.
_bounds_cache_valid = Bool(False)
_bounds_cache_valid = Bool(False, transient=True)

# Cached value of min and max as long as **data** doesn't change.
_bounds_cache = Tuple
_bounds_cache = Tuple(transient=True)

# ------------------------------------------------------------------------
# Public methods
Expand Down
6 changes: 3 additions & 3 deletions chaco/image_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ class ImagePlot(Base2DPlot):
# ------------------------------------------------------------------------

# Are the cache traits valid? If False, new ones need to be computed.
_image_cache_valid = Bool(False)
_image_cache_valid = Bool(False, transient=True)

# Cached image of the bmp data (not the bmp data in self.data.value).
_cached_image = Instance(GraphicsContextArray)
_cached_image = Instance(GraphicsContextArray, transient=True)

# Tuple-defined rectangle (x, y, dx, dy) in screen space in which the
# **_cached_image** is to be drawn.
_cached_dest_rect = Either(Tuple, List)
_cached_dest_rect = Either(Tuple, List, transient=True)

# Bool indicating whether the origin is top-left or bottom-right.
# The name "principal diagonal" is borrowed from linear algebra.
Expand Down
2 changes: 1 addition & 1 deletion chaco/label.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class Label(HasTraits):
# ------------------------------------------------------------------------

_bounding_box = List()
_position_cache_valid = Bool(False)
_position_cache_valid = Bool(False, transient=True)
_text_needs_fitting = Bool(False)
_line_xpos = Any()
_line_ypos = Any()
Expand Down
Loading