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

BF: Add missing opacity property to point actor #627

Merged
merged 1 commit into from Apr 14, 2015

Conversation

Projects
None yet
4 participants
@arybinski
Contributor

arybinski commented Apr 9, 2015

I'm playing with some example visualizations of points and lines, and noticed that changing opacity argument in point actor didn't work so I added it. It works, but it also shows some little artifacts in order of rendering when you use lower opacity---works as before with opacity=1. I'll explore the topic.

@arokem

This comment has been minimized.

Member

arokem commented Apr 9, 2015

Cool - will not have a chance to test it today, but hopefully early next week. Just a small comment on the process: we usually prefer to not have merge commits from master into feature branches. Rather, we prefer that developers use rebase:

git rebase master
git push -f origin branch-name

(Careful with that -f flag! See here: http://willi.am/blog/2014/08/12/the-dark-side-of-the-force-push/)

@arokem arokem referenced this pull request Apr 9, 2015

Closed

streamtubes opacity kwarg #459

@arokem

This comment has been minimized.

Member

arokem commented Apr 9, 2015

Also, you might want to also deal with other actors as well: #459

@arybinski

This comment has been minimized.

Contributor

arybinski commented Apr 9, 2015

Thanks for the link; I'm still getting familiar with the workflow so this will be helpful to read. I'll check out the other issue.

@arybinski

This comment has been minimized.

Contributor

arybinski commented Apr 14, 2015

Rebased, force pushed, and changed commit message according to the commit code.

@arokem

This comment has been minimized.

Member

arokem commented Apr 14, 2015

Great! Looks like you didn't destroy anything :-)

Don't worry about the Travis error. Sometimes it just misbehaves. I restarted that job, and hopefully it will come back green this time.

What about the opacity kwarg for the streamtubes?

@arybinski

This comment has been minimized.

Contributor

arybinski commented Apr 14, 2015

Force pushing felt like on the gifs in the blog entry you sent link to :)

Travis error is quite strange, because it affects only Python 3.2.

As I commented in streamtubes issue #459, opacity with streamtubes works on my machine.

@arokem

This comment has been minimized.

Member

arokem commented Apr 14, 2015

Yes. I just confirmed that, as you say, #459 is not really an issue. Closing that.

@arokem

This comment has been minimized.

Member

arokem commented Apr 14, 2015

I'll try running travis again, to see whether this is really a systematic
thing, or consistent.

On Tue, Apr 14, 2015 at 9:03 AM, Adam Rybinski notifications@github.com
wrote:

Force pushing felt like on the gifs in the blog entry you sent link to :)

Travis error is quite strange, because it affects only Python 3.2.

As I commented in streamtubes issues, opacity with streamtubes works on my
machine.


Reply to this email directly or view it on GitHub
#627 (comment).

@arokem

This comment has been minimized.

Member

arokem commented Apr 14, 2015

Still no joy from Travis. Anyone have any idea what's going on there, and why it's python 3.2 specific?

Whatever it is, it seems to also be happening in #629

@MrBago

This comment has been minimized.

Contributor

MrBago commented Apr 14, 2015

This is just a one line change? How can this be breaking travis?

@MrBago

This comment has been minimized.

Contributor

MrBago commented Apr 14, 2015

Master is also failing, https://travis-ci.org/MrBago/dipy/jobs/58498776. It seems that http://travis-wheels.scikit-image.org/ is providing a dead link for "numpy-1.9.1-cp32....whl".

@arokem

This comment has been minimized.

Member

arokem commented Apr 14, 2015

It worked just fine 9 days ago!
https://travis-ci.org/nipy/dipy/builds/57287789

Something must've happened to those wheels in the interim. IIRC, these were
masterfully built and tended to by @matthew-brett. Matthew - any ideas what
happened to the 3.2 wheels?

On Tue, Apr 14, 2015 at 1:24 PM, MrBago notifications@github.com wrote:

Master is also failing, https://travis-ci.org/MrBago/dipy/jobs/58498776.
It seems that http://travis-wheels.scikit-image.org/ is providing a dead
link for "numpy-1.9.1-cp32....whl".


Reply to this email directly or view it on GitHub
#627 (comment).

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Apr 14, 2015

I'm not sure what happened there - I've uploaded a fresh build of the wheels - try again?

@arokem

This comment has been minimized.

Member

arokem commented Apr 14, 2015

Yep. Thanks for that @matthew-brett, and thanks @arybinski for making the fix!

arokem added a commit that referenced this pull request Apr 14, 2015

Merge pull request #627 from arybinski/add_opacity_to_point
BF: Add missing opacity property to point actor

@arokem arokem merged commit bb47d2e into nipy:master Apr 14, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment