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

Adds a MultiBlock to store each plotted mesh. #24

Merged
merged 12 commits into from
Jul 5, 2021

Conversation

jeffreypaul15
Copy link
Collaborator

@jeffreypaul15 jeffreypaul15 commented Jun 23, 2021

MultiBlocks work as an array of meshes.
Field lines are added into a separate MultiBlock which is then added to the MultiBlock of the plot.
We need to find a way to refer to each mesh other than it's position in the MultiBlock, but I think that can be done later?

@nabobalis
Copy link
Contributor

nabobalis commented Jun 29, 2021

We need to find a way to refer to each mesh other than it's position in the MultiBlock, but I think that can be done later?

You mean by name?

Could you add a test for these changes please?

@jeffreypaul15
Copy link
Collaborator Author

You mean by name?

Yeah as of now once converted to a MultiBlock thery're part of an array, would it be wise to store a dict of all the meshes
corresponding to it's position in the array?

Could you add a test for these changes please?

Yeah, at the time none of the tests were written, I'll add them in today.

@nabobalis
Copy link
Contributor

Yeah as of now once converted to a MultiBlock thery're part of an array, would it be wise to store a dict of all the meshes
corresponding to it's position in the array?

I think its fine.

@jeffreypaul15
Copy link
Collaborator Author

I think its fine.

I'm sorry, I didn't quite get you, fine as in fine to go ahead with it?

@nabobalis
Copy link
Contributor

I think its fine.

I'm sorry, I didn't quite get you, fine as in fine to go ahead with it?

Yeah

sunkit_pyvista/plotter.py Outdated Show resolved Hide resolved
sunkit_pyvista/plotter.py Outdated Show resolved Hide resolved
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

Looks 👍 - the new all_meshes attribute should be documented in the docstring of SunpyPlotter. This can be done by adding an

Attributes
----------

section underneath the parameters section.

sunkit_pyvista/plotter.py Outdated Show resolved Hide resolved
sunkit_pyvista/plotter.py Outdated Show resolved Hide resolved
sunkit_pyvista/plotter.py Outdated Show resolved Hide resolved
sunkit_pyvista/plotter.py Outdated Show resolved Hide resolved
Co-authored-by: David Stansby <dstansby@gmail.com>
@nabobalis nabobalis requested a review from dstansby July 5, 2021 15:43
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.

3 participants