Pyvista #463

samjrholt · 2023-08-29T16:02:29Z

To-do

github-actions · 2023-08-29T16:02:50Z

CodeRabbit

Walkthrough

This update introduces a new feature for 3D visualization using the PyVista library. It adds pyvista properties to various classes, enabling direct access to PyVista-based plotting interfaces. New classes PyVistaMesh and PyVistaRegion are introduced for handling 3D mesh plotting and region visualization respectively.

Changes

File(s)	Summary
`discretisedfield/field.py`	Simplified array naming in `to_vtk` function and added `pyvista` property for PyVista-based plotting interface.
`discretisedfield/mesh.py`, `discretisedfield/region.py`	Introduced `pyvista` property returning instances of `dfp.PyVistaMesh` and `dfp.PyVistaRegion` respectively.
`discretisedfield/plotting/__init__.py`	Added `PyVistaMesh` and `PyVistaRegion` modules to the package.
`discretisedfield/plotting/pyvista_mesh.py`, `discretisedfield/plotting/pyvista_region.py`	Introduced `PyVistaMesh` and `PyVistaRegion` classes for 3D mesh plotting and region visualization. Provided customization methods for visualizations.

🐰🎉

In the land of code, where logic intertwines,
A rabbit hops forth, leaving tracks in the lines.
With each leap and bound, a change is unfurled,
Adding depth and dimension to our digital world.
Through meshes and regions, in 3D we now play,
Celebrating these changes in the PyVista way! 🎊🥳

Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

github-actions

CodeRabbit

Commits

Files that changed from the base of the PR and between e03dc36 and 113b49e commits.

Files selected (8)

dev/pyvista.ipynb (1)
discretisedfield/field.py (2)
discretisedfield/mesh.py (1)
discretisedfield/plotting/init.py (1)
discretisedfield/plotting/pyvista_field.py (1)
discretisedfield/plotting/pyvista_mesh.py (1)
discretisedfield/plotting/pyvista_region.py (1)
discretisedfield/region.py (1)

Files not summarized due to errors (1)

dev/pyvista.ipynb (diff tokens exceeds limit)

Files not reviewed due to errors (3)

discretisedfield/plotting/pyvista_field.py (no response)
dev/pyvista.ipynb (no response)
discretisedfield/plotting/init.py (no response)

Review comments generated (14)

Review: 12
LGTM: 2

Tips

Chat with CodeRabbit Bot (`@coderabbitai`)

Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

discretisedfield/field.py

discretisedfield/mesh.py

discretisedfield/plotting/pyvista_mesh.py

github-actions · 2023-09-14T09:26:11Z

discretisedfield/plotting/pyvista_mesh.py

+        if self.mesh.region.ndim != 3:
+            raise ValueError(
+                "Only meshes with 3 spatial dimensions can be plotted not"
+                f" {self.data.mesh.region.ndim=}."
+            )


CodeRabbit

Similarly, the dimension check in the subregions method is also unnecessary as it's already done in the __init__ method.

- if self.mesh.region.ndim != 3: - raise ValueError( - "Only meshes with 3 spatial dimensions can be plotted not" - f" {self.data.mesh.region.ndim=}." - )

I would agree with the automated check. Why do we need to repeat this check? It should have already been performed when initialising the class.

discretisedfield/plotting/pyvista_mesh.py

discretisedfield/plotting/pyvista_region.py

github-actions · 2023-09-14T09:26:12Z

discretisedfield/plotting/pyvista_region.py

+        if self.region.ndim != 3:
+            raise RuntimeError("Only 3-dimensional regions can be plotted.")


CodeRabbit

This check for the region's dimension is redundant as it was already checked during the initialization of the class. This can be removed to improve performance and reduce unnecessary code.

- if self.region.ndim != 3: - raise RuntimeError("Only 3-dimensional regions can be plotted.")

discretisedfield/plotting/pyvista_region.py

discretisedfield/plotting/pyvista_field.py

kzqureshi

It seems good to me, no specific typos as well, as per my vague notion of pyvista, code is very well written.

lang-m · 2024-01-12T13:58:22Z

@swapneelap Could you maybe review this so that we can move on. I can have a look at some later point (after merge) as I won't be able to (thoroughly) do this anytime soon.

swapneelap

Looks OK to me.

lang-m · 2024-01-15T09:11:44Z

@samjrholt Please resolve the issues pointed out by @kzqureshi . Afterwards you can merge unless you think we have to discuss any remaining issues (a few items in the list in the description are not yet ticked).

lang-m

Here is a very quick review. General remarks:

Looks like we use the default pyvista colormaps? I think that is a good decision.
We should have a (short) documentation notebook. Maybe the dev notebook could be modified for that purpose?

discretisedfield/io/vtk.py

discretisedfield/mesh.py

discretisedfield/plotting/pyvista_field.py

discretisedfield/plotting/util.py

pyproject.toml

discretisedfield/plotting/pyvista_field.py

discretisedfield/plotting/pyvista_mesh.py

lang-m · 2024-01-15T12:36:54Z

discretisedfield/plotting/pyvista_mesh.py

+        if self.mesh.region.ndim != 3:
+            raise ValueError(
+                "Only meshes with 3 spatial dimensions can be plotted not"
+                f" {self.data.mesh.region.ndim=}."
+            )


I would agree with the automated check. Why do we need to repeat this check? It should have already been performed when initialising the class.

Co-authored-by: Martin Lang <[email protected]>

…vista

samjrholt added 3 commits August 28, 2023 18:35

Region

d4c0fa7

Mesh

aa887cd

field

6745601

samjrholt marked this pull request as draft August 29, 2023 16:02

contour

170f4e7

samjrholt self-assigned this Aug 30, 2023

samjrholt added 2 commits August 30, 2023 18:29

clean up

e8f5bf6

pyvista

113b49e

github-actions bot reviewed Sep 14, 2023

View reviewed changes

samjrholt and others added 20 commits October 24, 2023 13:56

Merge branch 'master' into pyvista

26f45b0

plotting valid

7f9379c

remove legend background

bc7cf05

streamlines

b2f676b

scalar and plot to plotter

e030697

update mesh

b3f330a

clean up field

aacf349

plotter

9ddac01

glyphs

4b4a3cc

streamlines

1a69960

contour kwargs

3b10e07

save_screenshot

a3be03e

save multiple file types

af2c9ad

Cut scalars

61a7535

add valid plotting

b77ed51

Update pyproject.toml

a9668f9

vtk read

c755ce2

remove docstring test

3abb81a

Doc strings

aeba6ba

doc strings

18db167

samjrholt added 4 commits November 9, 2023 17:19

skip if not linux

3826a00

Only run streamlines on linux

5a642b6

Remove call

3a322f0

see-also docstring

b29a778

samjrholt requested a review from kzqureshi November 10, 2023 11:02

samjrholt marked this pull request as ready for review November 10, 2023 11:03

plotter typo

0828b71

kzqureshi reviewed Nov 21, 2023

View reviewed changes

discretisedfield/plotting/pyvista_field.py Show resolved Hide resolved

kzqureshi reviewed Nov 21, 2023

View reviewed changes

discretisedfield/plotting/pyvista_field.py Show resolved Hide resolved

kzqureshi reviewed Nov 21, 2023

View reviewed changes

swapneelap approved these changes Jan 15, 2024

View reviewed changes

Typo

307d8ca

lang-m reviewed Jan 15, 2024

View reviewed changes

samjrholt and others added 13 commits January 15, 2024 14:39

Typo and copy

9527770

Cell control and copy

82613d0

Copy and default colour

7df4560

Add pyvista docs

0307978

Update discretisedfield/io/vtk.py

4f39fb7

Co-authored-by: Martin Lang <[email protected]>

Docstring

20fe986

pyvista file saving

0849563

Extra backticks

9decaae

Remove superfluous error

cd8e2f0

Add comment on bug

cdd9711

Merge branch 'pyvista' of github.com:ubermag/discretisedfield into py…

2756e0a

…vista

remove typo

26921ee

remove copy

81aed6f

samjrholt merged commit 1b28a81 into master Mar 6, 2024
8 checks passed

samjrholt deleted the pyvista branch March 6, 2024 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pyvista #463

Pyvista #463

samjrholt commented Aug 29, 2023 •

edited

Loading

github-actions bot commented Aug 29, 2023 •

edited

Loading

CodeRabbit Pro

github-actions bot left a comment

github-actions bot Sep 14, 2023

lang-m Jan 15, 2024

github-actions bot Sep 14, 2023

kzqureshi left a comment

lang-m commented Jan 12, 2024

swapneelap left a comment

lang-m commented Jan 15, 2024

lang-m left a comment

lang-m Jan 15, 2024

		if self.region.ndim != 3:
		raise RuntimeError("Only 3-dimensional regions can be plotted.")

Pyvista #463

Pyvista #463

Conversation

samjrholt commented Aug 29, 2023 • edited Loading

To-do

github-actions bot commented Aug 29, 2023 • edited Loading

Walkthrough

Changes

CodeRabbit Pro

github-actions bot left a comment

Choose a reason for hiding this comment

Chat with CodeRabbit Bot (@coderabbitai)

Code suggestions

Pausing incremental reviews

github-actions bot Sep 14, 2023

Choose a reason for hiding this comment

lang-m Jan 15, 2024

Choose a reason for hiding this comment

github-actions bot Sep 14, 2023

Choose a reason for hiding this comment

kzqureshi left a comment

Choose a reason for hiding this comment

lang-m commented Jan 12, 2024

swapneelap left a comment

Choose a reason for hiding this comment

lang-m commented Jan 15, 2024

lang-m left a comment

Choose a reason for hiding this comment

lang-m Jan 15, 2024

Choose a reason for hiding this comment

samjrholt commented Aug 29, 2023 •

edited

Loading

github-actions bot commented Aug 29, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)