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

Add a module to make streamlines from an array of seeds #678

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Cadair
Copy link

@Cadair Cadair commented Jul 9, 2018

A long time ago, I needed to make streamlines from a specific set of points. This is my first attempt to graft it into mayavi.

It would be nice if it could be integrated into the main Streamline class rather than having to make a new one, but I don't know how to make the Streamline.seed attribute be either a SourceWidget or PolyData. I guess it would be possible to make a variant on SourceWidget which is just a wrapper around PolyData but that seems a little overkill.

@codecov-io
Copy link

codecov-io commented Jul 9, 2018

Codecov Report

Merging #678 into master will increase coverage by <.01%.
The diff coverage is 61.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #678      +/-   ##
==========================================
+ Coverage   50.59%   50.59%   +<.01%     
==========================================
  Files         257      257              
  Lines       23437    23455      +18     
  Branches     3194     3197       +3     
==========================================
+ Hits        11858    11868      +10     
- Misses      10819    10825       +6     
- Partials      760      762       +2
Impacted Files Coverage Δ
mayavi/modules/streamline.py 89.43% <61.9%> (-5.81%) ⬇️

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 501bab4...6525178. Read the comment docs.

Copy link
Member

@prabhuramachandran prabhuramachandran left a comment

Choose a reason for hiding this comment

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

Thanks for the PR but I think we could do this in a nicer way. Let me know what you think.

@@ -168,7 +171,8 @@ def update_pipeline(self):

src = mm.source
self.configure_connection(self.stream_tracer, src)
self.seed.inputs = [src]
if isinstance(self.seed, SourceWidget):
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR! I am not too happy about this isinstance check, it suggests that there should be a cleaner way to do this. Is the intent to just allow a user to set the polydata and not use a widget at all? I think it would be very useful thing in any case to allow users complete control over the points. So maybe I should think of supporting either a widget or a poly data. This could perhaps be done by simply having another attribute on the streamline module, say user_seed which is a polydata and if this is non-None, that we use that instead of the seed?

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

3 participants