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

Color Transfer Function editor gadget #124

Closed
wants to merge 9 commits into from
Closed

Conversation

jwiggins
Copy link
Member

@jwiggins jwiggins commented Mar 3, 2014

This big chunk of code implements a CTF editor for creating/editing functions which map from some data space to RGBA values. I'm using it for a volume rendering viewer, but it could also be used for creating a colormap for an image plot.

NOTE: This code needs needs traits-enaml [1] and a recent version of enaml[2], but since it's not imported anywhere, the dependency isn't explicit.

  1. traits-enaml https://github.com/enthought/traits-enaml
  2. enaml https://github.com/nucleic/enaml

@@ -0,0 +1,33 @@
from traits.etsconfig.api import ETSConfig
Copy link
Member

Choose a reason for hiding this comment

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

Demos should have an explanatory docstring.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add one.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.54%) when pulling 79c4403 on feature/ctf-gadget into a5b8cb9 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d4dcc58 on feature/ctf-gadget into a5b8cb9 on master.

@itziakos
Copy link
Member

itziakos commented Mar 4, 2014

It would be nice to have some tests to complement the code.

""" A value drag tool for editing a PiecewiseFunction.
"""

#: The function being edited
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: I think for constancy with the rest of the code #: -> #

@itziakos
Copy link
Member

itziakos commented Mar 4, 2014

I am not sure if this code belongs in enable since it practically contains more references and functionality from pyface more than enable. The feature also adds an additional dependency to traits-enaml and enaml which in my opinion is not good.

@jwiggins
Copy link
Member Author

jwiggins commented Mar 4, 2014

I'm open to suggestions about where this could go. I thought about it for a while before settling on enable. I think that it's useful to code which is using mayavi or chaco (and needs to create colormaps).

If I factor out the enaml bits in the way which you suggested, will this fit into enable better?

@rkern
Copy link
Member

rkern commented Mar 4, 2014

engadget.

@jwiggins
Copy link
Member Author

jwiggins commented Mar 4, 2014

rimshot

@jwiggins
Copy link
Member Author

jwiggins commented Mar 4, 2014

Wow, there's no indication that you edited your comment...

So, if we were to make a new ETS project called engadget, what would it contain aside from this?

@rkern
Copy link
Member

rkern commented Mar 4, 2014

Yeah, sorry. Everything in the angle brackets was blacklisted as malformed-html so I had to ninja-edit it to escape them. I do wish Github would send out notifications for edits.

I think there is a package's worth of potential UI widgets that are implemented in Enable/Chaco with the appropriate hooks for Traits UI and/or Enaml. For example, a double-ended slider that displays a range tool over a histogram/CDF of some dataset. Or a color selector in HuSL space and other interesting colorspaces.

@jwiggins
Copy link
Member Author

jwiggins commented Mar 4, 2014

So, like encore, but dependent on several other (all?) ETS packages instead of none of them?

I like that better than shoehorning this widget into enable. Just name it well!

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 40b83a2 on feature/ctf-gadget into a5b8cb9 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d6897ba on feature/ctf-gadget into a5b8cb9 on master.

@cfarrow
Copy link
Member

cfarrow commented Mar 5, 2014

Let the bikeshedding begin!

envelope - meh
enrich - better than envelope
enchilada - as in "the whole"
endeavor - I like enchilada better
engird - another meh
engage - some sort of gauge library, or gage library, either one really.
enthrall, enchant, enliven, enhance, etc. - a nice place for flashy examples
ensemble - like enchilada, but will probably be less annoying in the long run

My vote is for ensemble.

@jwiggins
Copy link
Member Author

jwiggins commented Mar 5, 2014

👍 ensemble

@jwiggins
Copy link
Member Author

jwiggins commented Mar 5, 2014

The tool behaves a bit strangely in a unit test, so I'm exercising the code but not testing it.

@itziakos
Copy link
Member

itziakos commented Mar 5, 2014

The tool behaves a bit strangely in a unit test, so I'm exercising the code but not testing it.

Can push the unit test in a parallel branch, I might be able to help.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.53%) when pulling f76603c on feature/ctf-gadget into a5b8cb9 on master.

@jwiggins
Copy link
Member Author

jwiggins commented Mar 5, 2014

I kinda think this PR should be closed and reopened on a shiny new ensemble repository.

@jwiggins
Copy link
Member Author

jwiggins commented Mar 7, 2014

To be continued at enthought/ensemble#1

@jwiggins jwiggins closed this Mar 7, 2014
@jwiggins jwiggins deleted the feature/ctf-gadget branch March 7, 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

5 participants