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

Added Examples #1584

Merged
merged 10 commits into from Sep 8, 2018

Conversation

Projects
5 participants
@karandeepSJ
Copy link
Contributor

karandeepSJ commented Jul 6, 2018

This PR adds examples for the new elements in viz.ui. Currently, this includes the examples for the merged elements. I will add more as the other PRs get merged.

@pep8speaks

This comment has been minimized.

Copy link

pep8speaks commented Jul 6, 2018

Hello @karandeepSJ, Thank you for updating !

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on August 12, 2018 at 15:09 Hours UTC

@dmreagan dmreagan added the gsoc2018 label Jul 11, 2018

@dmreagan dmreagan added this to PR needs a review in Viz Module Jul 17, 2018

@dmreagan

This comment has been minimized.

Copy link
Contributor

dmreagan commented Jul 20, 2018

I'm not sure we want a second UI example. I think I'd prefer to clean up the existing example and add these to it. By clean up, I mean:

  • make the window larger so you can see everything
  • line all the elements up in a nice grid

Something like this

@karandeepSJ karandeepSJ force-pushed the karandeepSJ:examples branch from c3f7c8d to 2c198b0 Aug 12, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Aug 12, 2018

Codecov Report

Merging #1584 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1584      +/-   ##
==========================================
+ Coverage   87.33%   87.34%   +0.01%     
==========================================
  Files         246      246              
  Lines       32177    32265      +88     
  Branches     3495     3504       +9     
==========================================
+ Hits        28102    28183      +81     
- Misses       3242     3247       +5     
- Partials      833      835       +2
Impacted Files Coverage Δ
dipy/viz/ui.py 89.2% <ø> (-0.01%) ⬇️
dipy/viz/tests/test_ui.py 82.14% <0%> (-0.4%) ⬇️
dipy/viz/actor.py 82.79% <0%> (+0.01%) ⬆️
dipy/viz/tests/test_actors.py 74.18% <0%> (+0.28%) ⬆️
dipy/workflows/tests/test_workflow.py 88.63% <0%> (+0.4%) ⬆️
dipy/viz/window.py 64.8% <0%> (+0.57%) ⬆️
dipy/workflows/multi_io.py 68.7% <0%> (+1.5%) ⬆️
dipy/viz/utils.py 61.67% <0%> (+5.98%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4890903...3170c80. Read the comment docs.

@karandeepSJ

This comment has been minimized.

Copy link
Contributor

karandeepSJ commented Aug 12, 2018

New Examples:
examples

@dmreagan

This comment has been minimized.

Copy link
Contributor

dmreagan commented Aug 14, 2018

Huge improvement over the previous mess! Ultimately we'll want this to replace viz_ui.py instead of adding a second file. We'll also want to edit the text so it provides a coherent tutorial for the user.

@karandeepSJ karandeepSJ force-pushed the karandeepSJ:examples branch from f1e7de3 to dd63aa2 Sep 6, 2018

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Sep 6, 2018

I feel like this PR is ready to go. Any other comment? I will wait until tomorrow for merging it

@skoudoro
Copy link
Member

skoudoro left a comment

Hi @karandeepSJ, it is almost ready to be merged. Can you fix the small comment below.

Thank you

First, a bunch of imports.
"""
import os

This comment has been minimized.

@skoudoro

skoudoro Sep 7, 2018

Member

can you separate """ and import os by an empty line.

We need it to render the documentation correctly

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Sep 8, 2018

Thank you @karandeepSJ and @dmreagan. Merging!

@skoudoro skoudoro merged commit 98495ba into nipy:master Sep 8, 2018

3 checks passed

codecov/patch Coverage not affected when comparing 4890903...3170c80
Details
codecov/project 87.34% (+0.01%) compared to 4890903
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Viz Module automation moved this from PR needs a review to Done Sep 8, 2018

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