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 stretch_data=False #227

Merged
merged 12 commits into from Oct 23, 2014
Merged

Fix stretch_data=False #227

merged 12 commits into from Oct 23, 2014

Conversation

corranwebster
Copy link
Member

This is a fix for #226 which includes (or supersedes) #225.

This pulls out the logic for the range adjustments into a utility method _adjust_range which is called from the low_pos and high_pos listeners in addition to the screen_bounds setter it was included in. It also removes the stray abs causing the traceback in #225.

To exercise this PR, you can run the following example script from @mdickinson :

from numpy import linspace
from scipy.special import jn

from chaco.api import ArrayPlotData, Plot
from enable.api import Component, ComponentEditor
from traits.api import HasTraits, Instance
from traitsui.api import Group, Item, View


class Demo(HasTraits):
    plot = Instance(Component)

    def _plot_default(self):
        x = linspace(-2.0, 10.0, 100)
        plot_data = ArrayPlotData(x=x, y=jn(3, x))
        plot = Plot(plot_data, origin="top left")
        plot.value_mapper.stretch_data = False
        plot.plot(("x", "y"))
        return plot

    traits_view = View(
        Group(
            Item(
                'plot',
                editor=ComponentEditor(),
                show_label=False,
            ),
        ),
        resizable=True,
    )


if __name__ == "__main__":
    demo = Demo()
    demo.configure_traits()

The stretch_data = False should lead to the data not being stretched along the y_axis. And this fixes the traceback that the above code leads as described in #225 .

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) when pulling d8cf8dc on fix/stretch-data-false into 3adeb8e on master.

@jonathanrocher
Copy link
Collaborator

Trying to follow what is going on. @corranwebster @mdickinson Does this PR make #225 obsolete?

@corranwebster
Copy link
Member Author

It's a more comprehensive fix for some further issues which were discovered, which includes @mdickinson's PR. You could either merge #225 and then this one, or just this one. Your choice.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) when pulling 22df7a7 on fix/stretch-data-false into 3adeb8e on master.

@jonathanrocher
Copy link
Collaborator

Thanks. Sorry I didn't realize that this one contained Mark's commits. Make sure to update the description of the PR with nice code snippets like @mdickinson . Pretty please...

@corranwebster
Copy link
Member Author

@jonathanrocher In this case the relevant scripts are in the issue that is being fixed: #226

@mdickinson
Copy link
Member

How about tests for the case where low_pos and high_pos are updated one after the other, but the orientation is reversed after the first assignment? (E.g., starting with low_pos = 0; high_pos = 100, then setting low_pos = 200 followed by high_pos = 300.)

Maybe this can't happen in normal use.

@corranwebster
Copy link
Member Author

That sort of thing could totally happen if the component was moved.

When the origin gets changed in the DataView or Plot, what happens there is even worse: it doesn't do a swap of high and low, but instead does a set of low_pos = old_high_pos, and then a separate high_pos = old_low_pos so that momentarily the mapper has a screen space of 0.

And everything (potentially) gets rendered twice because an update happened. sigh

@coveralls
Copy link

Coverage Status

Coverage increased (+0.22%) when pulling 61e0a25 on fix/stretch-data-false into 3adeb8e on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.25%) when pulling 61e0a25 on fix/stretch-data-false into 3adeb8e on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.27%) when pulling b1eab5a on fix/stretch-data-false into 3adeb8e on master.

@pberkes
Copy link
Contributor

pberkes commented Oct 17, 2014

This works well for our application, @tonysyu @noraderam @jwiggins please have a look to see that fixing the stretch does not break anything on your side.

@tonysyu
Copy link
Contributor

tonysyu commented Oct 17, 2014

As far as I can tell, we never set stretch_data to False and everything looks OK when running our app with this branch. 👍

@corranwebster
Copy link
Member Author

@jonathanrocher Unless there are any other concerns, I think this is ready to merge.

# Via Ioannis Tziakos (1) and Jonathan Rocher (1)
* master:
  Update cmap_image_plot.py
  no need for scipy
  update change log
  fix issue #220
  add a test for issue #220

Conflicts:
	CHANGES.txt
@coveralls
Copy link

Coverage Status

Coverage increased (+0.12%) when pulling b12e7e5 on fix/stretch-data-false into 0a151d9 on master.

@itziakos
Copy link
Member

@jonathanrocher, I have updated the PR and fixed the merged conflicts. I think this PR should be ready to merge

@jonathanrocher
Copy link
Collaborator

OK. Will do one last set of local testing and will merge today.

@itziakos
Copy link
Member

@jonathanrocher, did you have time to check the PR?

@jonathanrocher
Copy link
Collaborator

LGTM. Merging

jonathanrocher pushed a commit that referenced this pull request Oct 23, 2014
@jonathanrocher jonathanrocher merged commit 9a628ab into master Oct 23, 2014
@jonathanrocher jonathanrocher deleted the fix/stretch-data-false branch October 23, 2014 15:35
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.

None yet

7 participants