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

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

Closed
wants to merge 3 commits into from

Conversation

bnavigator
Copy link
Contributor

@bnavigator bnavigator commented Jan 26, 2024

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.

  • VTK deprecated some threshold filter methods in 9.1 and removed them in 9.3
  • There is a new attribute found by the parser
  • Somehow the test contour now returns slightly rounded off values.

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.

Copy link
Collaborator

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me!

@prabhuramachandran would you be up for making me a maintainer so I can at least merge PRs that keep things working for packaging like this and #1291 ? I wouldn't want to cut releases or anything like that but we could at least keep master green hopefully!

@prabhuramachandran
Copy link
Member

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
Copy link
Member

@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
Copy link
Collaborator

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 😱 . I promise to be careful, but if that's (understanably) too much power to delegate for now then "maintainer" is already good start!

@dpinte
Copy link
Member

dpinte commented Mar 11, 2024

@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
Copy link
Collaborator

larsoner commented Mar 11, 2024

@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
Copy link
Collaborator

@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
Copy link
Member

@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
Copy link
Collaborator

Superseded by #1291

@larsoner larsoner closed this Mar 14, 2024
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.

4 participants