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

Adds Sphere Object to sphere function in viz.actors #56

Open
wants to merge 61 commits into
base: cancer_spheres
from

Conversation

Projects
None yet
@thechargedneutron
Copy link

thechargedneutron commented May 18, 2018

This change is inspired from https://github.com/nipy/dipy/blob/6bee25c1d25acc1437752be0e166acce42bfed84/doc/examples/viz_surfaces.py#L92 .
I am not sure of this addition would override the existing polyData. Even if it does, we can merge the different things later.
I may be on a wrong track. Please correct me if I am going wrong.

Ping @Garyfallidis .

Garyfallidis and others added some commits May 12, 2018

Merge pull request nipy#1518 from jchoude/DOC_some_developers_affilia…
…tions

DOC: updated some developers affiliations.
Parichit Sharma
Updating the documentation for the workflow creation tutorial.
1) This change will be reflected on the page http://nipy.org/dipy/examples_built/workflow_creation.html#example-workflow-creation
2) Added one more import statement to import the AppendTextFlow method from the workflow class.
3) Added few lines to explain why we need to import the methods.
Parichit Sharma
Removed the trailing whitespaces as per the PEP8 standard.
1) The PEP8 still complains about the maximum line width but no complaints about the
trailing white spaces.
Parichit Sharma
Updated the workflow_creation.py according to the recommendations mad…
…e by Serge

1) Slightly modified the comments to tell that we are creating a separate file called as my_workflow.py in <dipy/workflows> folder.
2) Removed the first line about the import statements.
3) Comments are more precise now.
Merge pull request nipy#1521 from arokem/old_highlights
Moved some older highlights and announcements to the old news files.
Merge pull request nipy#1520 from parichit/doc_branch
Updating the documentation for the workflow creation tutorial.
@@ -1350,6 +1351,8 @@ def sphere(centers, colors, radii=1., theta=16, phi=16):
radii : float or ndarray, shape (N,)
theta : int
phi : int
sphere_obj : ndarray, shape (N,)

This comment has been minimized.

@Garyfallidis

Garyfallidis May 18, 2018

Owner

Should be a Sphere object.
or have different parameters for vertices and faces.

This comment has been minimized.

@thechargedneutron

thechargedneutron May 18, 2018

Author

But one object can only map to one sphere and we won't be able to map many spheres. Is this a desired behaviour?

This comment has been minimized.

@thechargedneutron

thechargedneutron May 21, 2018

Author

I guess array of objects suits our purpose. Having an object would make only one sphere. RIght?

@ranveeraggarwal

This comment has been minimized.

Copy link

ranveeraggarwal commented May 20, 2018

@Garyfallidis @thechargedneutron what if we need to visualise just Sphere objects? I can see two ways here:

  1. A separate function to visualise Sphere objects.
  2. Checks in the existing function
@thechargedneutron

This comment has been minimized.

Copy link
Author

thechargedneutron commented May 20, 2018

@ranveeraggarwal Yeah I forgot to consider the case when we do not pass anything except Sphere object. Both the solutions are legit and the implementation depends on the use case. Looks like check is an easy way out.

@Garyfallidis

This comment has been minimized.

Copy link
Owner

Garyfallidis commented May 20, 2018

@thechargedneutron, @ranveeraggarwal I had more thoughts about this. We shouldn't add a sphere object as one of the parameters. Because the viz parts will become an independent project. The Sphere object is strictly dipy related and will probably stay in dipy.
Create independent input variables vertices=None, faces=None and if they are not None then overrule theta and phi.

ShreyasFadnavis and others added some commits May 21, 2018

Merge pull request nipy#1529 from ShreyasFadnavis/micro
minor typo fix in quickstart
Wrong default value for parameter 'symmetric' False in connectivity_m…
…atrix function in tracking.utils was corrected as True.
Merge pull request nipy#1530 from albayenes/connectivity-matrix-wrong…
…-default-parameter-in-doc-for-symmetric

Wrong default value for parameter 'symmetric' connectivity_matrix function

MarcCote and others added some commits Apr 5, 2018

Jon Haitz Legarreta
DOC: Update Rafael's current institution.
Update Rafael's current institution.
Merge pull request nipy#1536 from jhlegarreta/UpdateRNHCurrentInstitu…
…tion

DOC: Update Rafael's current institution.
Merge pull request nipy#1538 from albayenes/fix-explanation-of-code-i…
…n-example-reconst-dki

Explanation that is mistakenly rendered as code fixed in example of DKI
Merge pull request nipy#1492 from MarcCote/enh_ui_components_position…
…ing2

Enh ui components positioning (with code refactoring)
Merge pull request nipy#1545 from Garyfallidis/readme_ref
Adding a reference in README.rst
Merge pull request nipy#1542 from Borda/fix_4_cvxpy
fix for using cvxpy solver
Merge pull request nipy#1540 from Borda/fix_div_zero
fix potential zero division in demon regist.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment