Consolidate overlays into "chaco.overlays" by aaronayres35 · Pull Request #741 · enthought/chaco

aaronayres35 · 2021-05-17T19:26:59Z

This PR would close #699

We still need to decide how we plan to deal with chaco.api and chaco.overlays.api (and also chaco.layers.api which will be moved into chaco/overlays/layers)

rahulporuri · 2021-05-18T12:32:43Z

We still need to decide how we plan to deal with chaco.api and chaco.overlays.api

IMO the recommendation will be to prefer chaco.api over chaco.overlays.api

... (and also chaco.layers.api which will be moved into chaco/overlays/layers)

Before we make that move/decision, we need to first know who uses chaco.layers.*. I don't know if there are any active users to that subpackage.

rahulporuri · 2021-05-18T12:35:36Z

The only additional comment i have regarding this draft PR is that the final PR will also need to search for and move the relevant tests to chaco.overlays.tests

aaronayres35 · 2021-05-19T14:51:30Z

+        # All other metadata is interpreted as a mask on dataspace
+        else:
+            ar = numpy.arange(0, len(selection), 1)
+            runs = arg_find_runs(ar[selection])


this flake8 error makes me suspect this code is actually broken:
chaco/overlays/layers/svg_range_selection_overlay.py:107:20: F821 undefined name 'arg_find_runs'

…error locally

aaronayres35 · 2021-05-24T21:14:44Z

The only additional comment i have regarding this draft PR is that the final PR will also need to search for and move the relevant tests to chaco.overlays.tests

It appears many are untested (at least not explicitly tested). Searching for test files containing Overlay yields only 2 results. I've moved the test_data_label.py file. I did not see others existing currently.

…ng deprecation warnigns. Also expose layers in the chaco.overlays.api

…ot get updated in merge of master

aaronayres35 · 2021-06-01T22:08:05Z

I had old files left as stubs in chaco/layers which raise deprecation warnings to preserve backwards compatibility. I also have the layers exposed in chaco.overlays.api, chaco.api, chaco.overlays.layers.api (this one really probably shouldn't exist?) and the now deprecated chaco.layers.api).

Also, currently the svg images in layers/data are duplicated, but perhaps the old set in chaco/layers/data can be removed

aaronayres35 · 2021-06-01T22:16:30Z

Before we make that move/decision, we need to first know who uses chaco.layers.*. I don't know if there are any active users to that subpackage.

I did a github search of chaco.layers and I only saw results in chaco itself. This is far from a guarantee though

aaronayres35 · 2021-06-01T22:17:10Z

I am going to convert this from a draft PR to a real PR now as we do aim to merge this going forward.

aaronayres35 · 2021-06-02T15:05:41Z

Note SvgRangeSelectionOverlay is not included in api modules as I think it is broken / it was not included in any api previously. See comment #741 (comment)
I am happy to add it if you think it should be exposed @rahulporuri
Maybe, if it is indeed broken / if we search and find it unused, it could be removed or deprecated.

rahulporuri

LGTM

rahulporuri · 2021-06-02T15:22:55Z

@aaronayres35 looks like grid and axis aren't included in this refactor - which are mentioned in the issue. I don't remember if we had already discussed why they aren't included.

aaronayres35 · 2021-06-02T15:24:53Z

@aaronayres35 looks like grid and axis aren't included in this refactor - which are mentioned in the issue. I don't remember if we had already discussed why they aren't included.

I don't recall if we discussed it either, all I remember was Corran's comment: #699 (comment)
Perhaps we should bring them in with the scale and tick generation code?

EDIT:
as we mentioned earlier today I think it may be okay to leave axis and grid out. They are a somewhat distinct from other overlays - they are underlays as described here:

chaco/chaco/plot_component.py

Lines 41 to 59 in a600fc2

    
           #: The order in which various rendering classes on this component are drawn. 
        
           #: Note that if this component is placed in a container, in most cases 
        
           #: the container's draw order is used, since the container calls 
        
           #: each of its contained components for each rendering pass. 
        
           #: Typically, the definitions of the layers are: 
        
           #: 
        
           #: 1. 'background': Background image, shading 
        
           #: 2. 'image': A special layer for plots that render as images.  This is in 
        
           #:    a separate layer since these plots must all render before non-image 
        
           #:    plots. 
        
           #: 3. 'underlay': Axes and grids 
        
           #: 4. 'plot': The main plot area itself 
        
           #: 5. 'selection': Selected content are rendered above normal plot elements 
        
           #:    to make them stand out 
        
           #: 6. 'border': Plot borders 
        
           #: 7. 'annotation': Lines and text that are conceptually part of the "plot" 
        
           #:    but need to be rendered on top of everything else in the plot 
        
           #: 8. 'overlay': Legends, selection regions, and other tool-drawn visual 
        
           #:    elements

rahulporuri · 2021-06-02T15:34:31Z

as we mentioned earlier today I think it may be okay to leave axis and grid out. They are a somewhat distinct from other overlays - they are underlays as described here:

SGTM. Go ahead with this PR and the rest of the rc process.

* copy modules into chaco/overlays * make original modules stubs raising deprecation warnings * copy layers into overlays * fully populate chaco.overlays.api and chaco.api * flake8 * update tox.ini by running flake8 -q * somehow this is also needed in tox.ini but I did not see this flake8 error locally * remove bad trailing comma * move data label test to sit in chaco/overlays/tests * add stub modules in chaco/layers importing from new location and adding deprecation warnigns. Also expose layers in the chaco.overlays.api * add layers to the chaco.api as well * old files that were moved have since been changed on master and did not get updated in merge of master * flake8 * typo * update deprecation warning messages * remove stub modules from autogenerated api docs * remove chaco/layers/data * import from chaco.plots in overlay modules * import Legend from chaco.overlays.api

* FIX: fix scrollbar demo and update docstring (#489) Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com> * Add new chaco.plots submodule (#738) * add new chaco.plots submodule * move contour_line_plot.py and contour_poly_plot.py into chaco/plots/contour * import from api in tests * move relevant tests into chaco/plots/tests * add stub files at old file locations which raise Deprecation warning and import objects from their new chaco/plots modules * flake8 * update warning message * set stacklevel=2 in deprecation warnings * add a chaco.plots.api * typo * update deprecation warning messages, and remove module docstrings * Apply suggestions from code review Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com> * ignore stub modules when generating api docs * flake8 * update api module docstrings * reorder imports * suggestions from code review Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com> * add pyproject.toml to MANIFEST.in (#747) * Consolidate overlays into "chaco.overlays" (#741) * copy modules into chaco/overlays * make original modules stubs raising deprecation warnings * copy layers into overlays * fully populate chaco.overlays.api and chaco.api * flake8 * update tox.ini by running flake8 -q * somehow this is also needed in tox.ini but I did not see this flake8 error locally * remove bad trailing comma * move data label test to sit in chaco/overlays/tests * add stub modules in chaco/layers importing from new location and adding deprecation warnigns. Also expose layers in the chaco.overlays.api * add layers to the chaco.api as well * old files that were moved have since been changed on master and did not get updated in merge of master * flake8 * typo * update deprecation warning messages * remove stub modules from autogenerated api docs * remove chaco/layers/data * import from chaco.plots in overlay modules * import Legend from chaco.overlays.api Co-authored-by: Xiaoyu Wu <xiaoyu-wu@users.noreply.github.com> Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>

aaronayres35 added 3 commits May 17, 2021 13:06

copy modules into chaco/overlays

5078f1f

make original modules stubs raising deprecation warnings

cf940ba

copy layers into overlays

82276fb

aaronayres35 changed the title ~~Consolidate overlays into "chaco.overlays"~~ [WIP] Consolidate overlays into "chaco.overlays" May 17, 2021

fully populate chaco.overlays.api and chaco.api

724469e

aaronayres35 requested a review from rahulporuri May 19, 2021 13:58

aaronayres35 added 2 commits May 19, 2021 09:42

flake8

4c4240e

update tox.ini by running flake8 -q

959d7c7

aaronayres35 commented May 19, 2021

View reviewed changes

aaronayres35 added 3 commits May 19, 2021 09:54

somehow this is also needed in tox.ini but I did not see this flake8 …

d411049

…error locally

remove bad trailing comma

a25de20

move data label test to sit in chaco/overlays/tests

7e4736a

aaronayres35 mentioned this pull request Jun 1, 2021

Chaco Release 5.0.0 #745

Closed

44 tasks

aaronayres35 and others added 4 commits June 1, 2021 16:55

add stub modules in chaco/layers importing from new location and addi…

eac7831

…ng deprecation warnigns. Also expose layers in the chaco.overlays.api

add layers to the chaco.api as well

f40b9ec

Merge branch 'master' into overlays-reorg

5118c58

old files that were moved have since been changed on master and did n…

ce3a087

…ot get updated in merge of master

aaronayres35 added 2 commits June 1, 2021 17:08

flake8

6102ff3

typo

35b365e

aaronayres35 changed the title ~~[WIP] Consolidate overlays into "chaco.overlays"~~ Consolidate overlays into "chaco.overlays" Jun 1, 2021

aaronayres35 marked this pull request as ready for review June 1, 2021 22:17

rahulporuri reviewed Jun 2, 2021

View reviewed changes

aaronayres35 added 2 commits June 2, 2021 09:50

update deprecation warning messages

76d65aa

remove stub modules from autogenerated api docs

aeb598e

remove chaco/layers/data

ecda2bb

aaronayres35 requested a review from rahulporuri June 2, 2021 15:05

rahulporuri approved these changes Jun 2, 2021

View reviewed changes

aaronayres35 mentioned this pull request Jun 2, 2021

Remove deprecated stub modules in chaco 6.0 #748

Closed

aaronayres35 and others added 3 commits June 2, 2021 10:42

Merge branch 'master' into overlays-reorg

4b97d56

import from chaco.plots in overlay modules

db01d7b

import Legend from chaco.overlays.api

582f076

aaronayres35 merged commit 2899f73 into master Jun 2, 2021

aaronayres35 deleted the overlays-reorg branch June 2, 2021 16:02

Conversation

aaronayres35 commented May 17, 2021

Uh oh!

rahulporuri commented May 18, 2021

Uh oh!

rahulporuri commented May 18, 2021

Uh oh!

aaronayres35 May 19, 2021

Choose a reason for hiding this comment

Uh oh!

aaronayres35 commented May 24, 2021

Uh oh!

aaronayres35 commented Jun 1, 2021

Uh oh!

aaronayres35 commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronayres35 commented Jun 1, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aaronayres35 commented Jun 2, 2021

Uh oh!

rahulporuri left a comment

Choose a reason for hiding this comment

Uh oh!

rahulporuri commented Jun 2, 2021

Uh oh!

aaronayres35 commented Jun 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rahulporuri commented Jun 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aaronayres35 commented Jun 1, 2021 •

edited

Loading

aaronayres35 commented Jun 2, 2021 •

edited

Loading