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

Bump PyVista for new DynamicScraper features #632

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

banesullivan
Copy link
Collaborator

Resolves #606.

This bumps the version of PyVista and makes some minor changes so that we can leverage PyVista's latest features in its DynamicScraper

@github-actions github-actions bot added type: documentation Auto-labelled for doc/* and docs/* branches type: dependencies Auto-labelled type: examples Auto-labelled for ex/*, example/* and examples/* branches labels Jan 10, 2024
@banesullivan
Copy link
Collaborator Author

FYI @bjlittle, I noticed render_points_as_spheres=True is used some places here. This may display oddly in the docs as pointed out in MatthewFlamm/pyransame#136 (comment)

Copy link

codecov bot commented Jan 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (46367f6) 94.93% compared to head (78e26da) 94.93%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #632      +/-   ##
==========================================
- Coverage   94.93%   94.93%   -0.01%     
==========================================
  Files         125      125              
  Lines        4779     4778       -1     
==========================================
- Hits         4537     4536       -1     
  Misses        242      242              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bjlittle
Copy link
Owner

@banesullivan Lovely! Thanks!

I just need to release 0.4.1 and merge back the changes in the 0.4.x release feature branch to main.

Then you can rebase and we can bank this 🍻

@bjlittle bjlittle self-requested a review January 10, 2024 10:39
@bjlittle bjlittle self-assigned this Jan 10, 2024
Copy link
Owner

@bjlittle bjlittle left a comment

Choose a reason for hiding this comment

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

@banesullivan

0.4.1 has now been released and the 0.4.x feature branch has been merged back to main (#633).

Fancy rebasing from main, then we can bank this 👍

Thanks!

@bjlittle
Copy link
Owner

bjlittle commented Jan 11, 2024

FYI @bjlittle, I noticed render_points_as_spheres=True is used some places here. This may display oddly in the docs as pointed out in MatthewFlamm/pyransame#136 (comment)

Is this using vtk.js under the hood to render? Just curious ... 🤔

I noticed that "text" also isn't supported e.g., titles or point labels.

Do you know if there are plans to support these at a later date?

@bjlittle
Copy link
Owner

bjlittle commented Jan 11, 2024

@banesullivan @tkoyama010

O--M--G! 😍🚀💯

I soooo love it! Insane!

image

@bjlittle
Copy link
Owner

@banesullivan BTW is there a way to control the size of the Interactive Scene rendered window?

@tkoyama010
Copy link
Collaborator

@bjlittle
Awesome! By the way, the preview of the RTD that you set up at #391 is not showing up in CI, do you know why?

@bjlittle
Copy link
Owner

bjlittle commented Jan 11, 2024

@bjlittle Awesome! By the way, the preview of the RTD that you set up at #391 is not showing up in CI, do you know why?

Yes. I had to temporarily disable it whilst I was working on the v0.4.x release feature branch. This is because v0.4.x is based on geovista 0.4.0, and this is before the documentation was building successfully (after my recent work on it).

I didn't want failing RTD CI tasks on the v0.4.x pull-requests.

I've since enabled it again after deploying 0.4.1.

@bjlittle
Copy link
Owner

If I close and then re-open this pull request, the RTD CI may kick-in again.

Let's try and see ...

@bjlittle bjlittle closed this Jan 11, 2024
@bjlittle bjlittle reopened this Jan 11, 2024
@bjlittle
Copy link
Owner

bjlittle commented Jan 11, 2024

Yup, that did the trick 👍

image

@tkoyama010
Copy link
Collaborator

@bjlittle Thanks. WFM.

@bjlittle
Copy link
Owner

bjlittle commented Jan 11, 2024

The pre-commit.ci job will remain Pending until the merge conflict is resolved.

Neat.

Didn't realise that pre-commit.ci detected this on a pull-request.

image

@banesullivan
Copy link
Collaborator Author

@bjlittle, the conflict is:

<<<<<<< banesullivan/bump-pyvista
  - pyvista >=0.40
=======
  - pyvista >=0.43.1
>>>>>>> main

Which do you want to support as the minimum version of PyVista?

@bjlittle
Copy link
Owner

Go with >=0.43.1 please.

Thanks 👍

Copy link
Collaborator

@tkoyama010 tkoyama010 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Owner

@bjlittle bjlittle left a comment

Choose a reason for hiding this comment

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

Awesome, thanks 🚀

@bjlittle bjlittle merged commit 416e656 into main Jan 11, 2024
17 checks passed
@bjlittle bjlittle deleted the banesullivan/bump-pyvista branch January 11, 2024 06:52
@bjlittle
Copy link
Owner

@all-contributors please add @banesullivan for infra and examples

Copy link
Contributor

@bjlittle

I've put up a pull request to add @banesullivan! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: dependencies Auto-labelled type: documentation Auto-labelled for doc/* and docs/* branches type: examples Auto-labelled for ex/*, example/* and examples/* branches
Projects
Status: 🏁 Done
Development

Successfully merging this pull request may close these issues.

Dynamically load interactive examples in documentation
3 participants