search

enthought / mayavi

3D visualization of scientific data in Python
http://docs.enthought.com/mayavi/mayavi/
Other
1.28k stars 282 forks source link

Port changes from upstream QVTKRWI. #1300

Closed prabhuramachandran closed 1 month ago

prabhuramachandran commented 1 month ago

This ports several changes from upstream as a result this works well with PyQt6.

larsoner commented 1 month ago

The macos-latest failure can be fixed by using the macos-13 image it's meant to use. -latest recently changed to -14 and that's ARM64 rather than Intel so other changes are likely needed. (In principle we want both, but for expediency it's easiest to change here).

Then ideally some merge requirements could be set for the CIs (i.e., allow "merge when green" button clicking).

The other failure is probably an issue with PySide 6.7 so you could pin to PySide!=6.7 if you want, or try to fix it.

prabhuramachandran commented 1 month ago

The macos-latest failure can be fixed by using the macos-13 image it's meant to use. -latest recently changed to -14 and that's ARM64 rather than Intel so other changes are likely needed. (In principle we want both, but for expediency it's easiest to change here).

Thanks, I am trying that now.

Then ideally some merge requirements could be set for the CIs (i.e., allow "merge when green" button clicking).

Yes, the interface expects some tests in the last week so I wasn't able to add those.

The other failure is probably an issue with PySide 6.7 so you could pin to PySide!=6.7 if you want, or try to fix it.

It fails even with older versions and looks to be a bit bizarre. For now I am skipping this test with pyside6.

Can you please review the PR.

prabhuramachandran commented 1 month ago

@larsoner -- could you please review the PR once so I can merge this and start the release?

larsoner commented 1 month ago

I would merge, but it looks like my approval is insufficient to enable the green button

prabhuramachandran commented 1 month ago

I would merge, but it looks like my approval is insufficient to enable the green button

@dpinte @mdickinson any ideas on how this could be done?

mdickinson commented 1 month ago

Hi @prabhuramachandran. I've (temporarily?) disabled the "Require approvals" settings in the branch protections for master, so that this PR can be merged. We should discuss (possibly by email) a longer-term solution.

tkoyama010 commented 1 month ago

Hi @prabhuramachandran. I've (temporarily?) disabled the "Require approvals" settings in the branch protections for master, so that this PR can be merged. We should discuss (possibly by email) a longer-term solution.

We has set up a GitHub Bot so that when an admin user makes an LGTM comment on a PR of which they are the author, the GitHub Bot approves that PR. This allows admins to merge their PRs as they wish, while leaving the rule that admins approve other PRs. https://github.com/pyvista/pyvista/pull/4941 Edit: I can create PR for this if you want :)

prabhuramachandran commented 1 month ago

Hi @prabhuramachandran. I've (temporarily?) disabled the "Require approvals" settings in the branch protections for master, so that this PR can be merged. We should discuss (possibly by email) a longer-term solution.

Thank you, yes let us discuss this via email. For now this is fine.

prabhuramachandran commented 1 month ago

@tkoyama010 -- Thank you for the offer. I am not sure about this and how this would work. Let me first discuss this and let you know. Thank you for the kind offer though.

mdickinson commented 1 month ago

Thank you, yes let us discuss this via email.

Email sent ... Let me know if you didn't receive anything.

prabhuramachandran commented 1 month ago

Thank you, yes let us discuss this via email.

Email sent ... Let me know if you didn't receive anything.

Got it, thank you. Sorry for the slow response.