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

NF: Added support to colorize each line points indivdually #942

Merged
merged 3 commits into from Nov 3, 2016

Conversation

Projects
None yet
5 participants
@gauvinalexandre
Contributor

gauvinalexandre commented Feb 28, 2016

No description provided.

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Feb 29, 2016

LGTM

@gauvinalexandre

This comment has been minimized.

Contributor

gauvinalexandre commented Feb 29, 2016

Half a day to find those 5 lines. Damn.

@arokem

This comment has been minimized.

Member

arokem commented Oct 21, 2016

I am not sure what's going on here, but it looks like @MarcCote gave this the thumbs-up months ago. Unless someone objects, I can merge this early next week.

@gauvinalexandre

This comment has been minimized.

Contributor

gauvinalexandre commented Oct 21, 2016

Thank you!

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Oct 21, 2016

@gauvinalexandre if you could add some information about this new feature in the docstring that would be nice. Here is a suggestion: "If an array (K, 3) is given, where K is the number of points of all lines then every point is colored with a different RGB color."

I remembered testing it and it was working fine. I should have merged it a while back, so early next week sounds good.

@gauvinalexandre

This comment has been minimized.

Contributor

gauvinalexandre commented Oct 21, 2016

Thanks @MarcCote ! I would add some info, but unfortunately I don't have much time to go back in the code and explain the use cases and show examples... If this is necessary before the merge, maybe we should wait... I'll add this to my short term todo list.

@arokem

This comment has been minimized.

Member

arokem commented Oct 25, 2016

No need for examples here, in my opinion (though we would really welcome
that, in a separate PR). Just the addition of that sentence into the
docstring.

On Fri, Oct 21, 2016 at 9:23 AM, Alexandre Gauvin notifications@github.com
wrote:

Thanks @MarcCote https://github.com/MarcCote ! I would add some info,
but unfortunately I don't have much time to go back in the code and explain
the use cases and show examples... If this is necessary before the merge,
maybe we should wait... I'll add this to my short term todo list.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#942 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAHPNtzpOQYl6-DYhwihayrHUlOk_Tdtks5q2ObqgaJpZM4Hks-g
.

@arokem

This comment has been minimized.

Member

arokem commented Oct 25, 2016

I second the suggestion to document this! I'll wait for that to come in and
then merge.

On Fri, Oct 21, 2016 at 9:16 AM, Marc-Alexandre Côté <
notifications@github.com> wrote:

@gauvinalexandre https://github.com/gauvinalexandre if you could add
some information about this new feature in the docstring that would be
nice. Here is a suggestion: "If an array (K, 3) is given, where K is the
number of points of all lines then every point is colored with a different
RGB color."

I remembered testing it and it was working fine. I should have merged it a
while back, so early next week sounds good.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#942 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAHPNsGmfMahfdcvCr9XIuE8wmI7pdKfks5q2OVFgaJpZM4Hks-g
.

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Oct 31, 2016

@gauvinalexandre just merge this PR that I've made on top of your branch: gauvinalexandre#1

Merge pull request #1 from MarcCote/viz_point_color_support
Mention new feature in docstring
@gauvinalexandre

This comment has been minimized.

Contributor

gauvinalexandre commented Nov 3, 2016

Thanks @MarcCote !

@coveralls

This comment has been minimized.

coveralls commented Nov 3, 2016

Coverage Status

Changes Unknown when pulling cf8ca6e on gauvinalexandre:viz_point_color_support into * on nipy:master*.

@codecov-io

This comment has been minimized.

codecov-io commented Nov 3, 2016

Current coverage is 85.44% (diff: 80.00%)

No coverage report found for master at cf3b892.

Powered by Codecov. Last update cf3b892...cf8ca6e

@arokem

This comment has been minimized.

Member

arokem commented Nov 3, 2016

I'm going to ignore the codecov weirdness here, and go ahead with the merge

@arokem arokem merged commit 7302303 into nipy:master Nov 3, 2016

2 of 4 checks passed

codecov/patch 80.00% of diff hit (target 85.44%)
Details
codecov/project 85.44% (-0.01%) compared to e8eeb70
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 87.982%
Details
@arokem

This comment has been minimized.

Member

arokem commented Nov 3, 2016

Thanks @gauvinalexandre !

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Nov 3, 2016

Probably a rebase was needed since we drop the coverage of benchmark files.

@arokem

This comment has been minimized.

Member

arokem commented Nov 3, 2016

That's probably it. Looks like it merged cleanly anyway...

@gauvinalexandre gauvinalexandre deleted the gauvinalexandre:viz_point_color_support branch Nov 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment