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

FIX: Circular imports in toolbar_plot and transform_color_mapper #469

Merged
merged 7 commits into from
Jul 18, 2019

Conversation

achabotl
Copy link
Contributor

Import directly from the module where each class is implemented instead of importing from chaco.api.

The fix is to import ColorMapper directly from chaco.color_mapper
@achabotl achabotl mentioned this pull request Jul 15, 2019
@jvkersch
Copy link
Contributor

Looks good; why not go the whole hog and prefer direct module imports over imports from chaco.api everywhere?

$ rg -g '!tests' "chaco.api"
function_image_data.py
3:from chaco.api import DataRange2D, ImageData

overlays/container_overlay.py
10:from chaco.api import PlotComponent

overlays/coordinate_line_overlay.py
11:from chaco.api import AbstractOverlay

overlays/databox.py
7:from chaco.api import AbstractOverlay

tools/dataprinter.py
11:from chaco.api import BaseXYPlot

tools/highlight_tool.py
11:from chaco.api import BasePlotContainer

tools/image_inspector_tool.py
9:from chaco.api import AbstractOverlay, ImagePlot, TextBoxOverlay

tools/line_inspector.py
10:from chaco.api import BaseXYPlot, Base2DPlot

tools/range_selection_overlay.py
12:from chaco.api import AbstractOverlay, arg_find_runs, GridMapper, AbstractMapper

tools/draw_points_tool.py
11:from chaco.api import ArrayDataSource

tools/traits_tool.py
9:from chaco.api import PlotAxis, ColorBar

tools/line_segment_tool.py
13:from chaco.api import AbstractOverlay

tools/range_selection.py
12:from chaco.api import AbstractController

tools/save_tool.py
59:        from chaco.api import PlotGraphicsContext

tools/regression_lasso.py
14:from chaco.api import LassoOverlay, Label

tools/lasso_selection.py
13:from chaco.api import AbstractController, AbstractDataSource, \

shell/plot_maker.py
16:from chaco.api import (create_line_plot, create_scatter_plot,

layers/status_layer.py
7:from chaco.api import AbstractOverlay

layers/svg_range_selection_overlay.py
7:from chaco.api import GridMapper

shell/scaly_plot.py
6:from chaco.api import (DataRange2D, LinearMapper, LogMapper,

shell/commands.py
14:from chaco.api import Plot, color_map_name_dict
800:        from chaco.api import PlotGraphicsContext

tests_with_backend/test_highlight_tool.py
8:from chaco.api import Plot, ArrayPlotData

tests_with_backend/test_2d_case.py
3:from chaco.api import Plot, ArrayPlotData

ui/popupable_plot.py
3:from chaco.api import VPlotContainer

@achabotl
Copy link
Contributor Author

The hog is now whole. No more chaco.api imports anywhere. No more chaco.tools.api and chaco.scales.api either.

Ready for review.

@jvkersch jvkersch self-requested a review July 17, 2019 10:45
Copy link
Contributor

@jvkersch jvkersch left a comment

Choose a reason for hiding this comment

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

That's great, thanks for doing this! There are some import errors from chaco.scales, but presumably those can be fixed easily.

@achabotl
Copy link
Contributor Author

Yes, that was an easy fix. I got bitten by the fact that python etstools.py test tests an installed version of Chaco (into the test env), and not an editable version.

Copy link
Contributor

@jvkersch jvkersch left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @achabotl !

@jvkersch jvkersch merged commit 785f180 into master Jul 18, 2019
@jvkersch jvkersch deleted the fix/circular-imports branch July 18, 2019 08:35
This was referenced Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants