search

enthought / mayavi

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

VTK 9.3: Fix test suite failures (VTK API changes) #1290

Closed bnavigator closed 3 months ago

bnavigator commented 5 months ago

We encounter a few test suite errors in the rpm build for Mayavi since VTK has been updated to 9.3.0 in openSUSE Tumbleweed.

This is by far not complete in terms of deprecations and removals for 9.3 (See #1286 for example), but at least it greens the test suite for us.

prabhuramachandran commented 4 months ago

Hi @larsoner -- I am terribly sorry I have not been able to do much. I have a real baby these days. And over the last couple of weeks have been traveling so have had little or no time for this. I would be very happy to have you become a maintainer. Thank you for asking and all your work!

prabhuramachandran commented 4 months ago

@larsoner - I am not sure what the correct way of making you a maintainer is but will ping the folks at Enthought. @dpinte , @rahulporuri -- Can you please add @larsoner as a maintainer? Thanks!

larsoner commented 4 months ago

Can you please add @larsoner as a maintainer? Thanks!

Whatever scope would allow me to merge PRs (like this), change branch protection rules (which need to change with #1291), and cut releases would be good. From the GitHub docs it sounds like this would be admin rather than maintainer at least for the branch protection part :scream: . I promise to be careful, but if that's (understanably) too much power to delegate for now then "maintainer" is already good start!

dpinte commented 3 months ago

@prabhuramachandran Eric has been added as maintainer on the project. @larsoner you now have maintainer rights on the repo! Once you've accepted the invite, we'll be able to tweak the branch protection to allow you to merge the PRs (that won't require you to be an admin, I think).

larsoner commented 3 months ago

@bnavigator okay with you for me to close this one and give you co-author credit on #1291 ? CIs are green over there so it's a bit simpler that way

larsoner commented 3 months ago

@dpinte yes I have a merge button now, which is nice. As expected, though, I can't change the branch protection rules. Can you update the branch protection rules to reflect the ones in #1291 ? FWIW I'd expect these to need updates every 6-12 months based on new Python and VTK versions coming out. There will most likely also be a one-off special update required in the next 6 weeks because the Cythonized file we ship will probably need to be updated (i.e., re-Cythonized with an updated Cython) to be NumPy 2.0 API compliant as it's not backward compatible and that should probably be a separate CI run (though strictly speaking doesn't have to be I suppose).

And then I'll also need a review approval to merge #1291 (or you could give me the "bypass branch protections" permissions in my role) as any PR requires 1 approval. This will be the case for any other PRs I open, so could slow things down since the primary problem here seems to be getting other reviewers/maintainers. (If I open a PR I can't be the one to also approve it.)

As long as there is someone I can ping for branch protection rule updates and PR reviews things should run smoothly. If there isn't... then I fear we'll end up with PRs getting stuck quite a bit again :(

prabhuramachandran commented 3 months ago

@larsoner -- if you need something from me, please feel free to ping me here and I can share other means of contacting me. Last month was difficult but going forward I think at least over the weekends I should be able to make some time for maintenance.

larsoner commented 3 months ago

Superseded by #1291