search

enthought / mayavi

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

Add a pyproject.toml. #1183

Closed prabhuramachandran closed 1 year ago

prabhuramachandran commented 1 year ago

@larsoner -- I am adding a pyproject.toml to see if it helps. Would be good if you could take a quick look. Thanks!

prabhuramachandran commented 1 year ago

@larsoner -- this is rubbish and doesn't work, so I will need to research this a bit more.

larsoner commented 1 year ago

I thought about doing it because I recently did it for codespell and it wasn't too bad. But the presence of the C extension will make it harder here I think

prabhuramachandran commented 1 year ago

@larsoner -- the problem was the building of the tvtk_classes.zip. I think I have "fixed" this for now and the builds mostly pass. This should hopefully fix the build and pep517 issues for now (until numpy distutils goes away). I will look into the one failure in the macos builds and push a fix for that.

prabhuramachandran commented 1 year ago

OK all tests have passed now.

rahulporuri commented 1 year ago

You can see that setup.py ends up being a 2-line convenience wrapper, and that all its contents effectively land in pyproject.toml. I'm not sure how we would do this with Mayavi, but it would be cool if we could someday!

@larsoner I imagine splitting tvtk into its' own package would help us move in that direction. We can publish tvtk wheels and make it a build dependency for mayavi. It might also help with keeping up with newer versions of VTK. Mayavi will continue using a working version of tvtk while we update it to work with a newer version. Correct me if I'm missing anything.

larsoner commented 1 year ago

Agreed splitting into two packages sounds more maintainable!

rahulporuri commented 1 year ago

@prabhuramachandran I keep comparing tvtk+mayavi to kiva+enable in my head but kiva doesn't need as much upkeep as tvtk does, from what I've seen over the past 3 years.

larsoner commented 1 year ago

Yeah, I wouldn't be at all surprised if Mayavi code from today worked on tvtk+VTK from 5 years ago. It seems like the core of Mayavi has been pretty stable and updates have been to the Qt bindings, which for the most part so far have been separate from VTK/tvtk updates. But I haven't been paying very close attention so I might be wrong about this

prabhuramachandran commented 1 year ago

@larsoner, @rahulporuri -- yes I think we can clean up the setup.py and remove a bunch of cruft, we can also use the pyproject.toml along with the customization options for setuptools to build the ZIP files too. There was a time when tvtk was a separate package but it was a bit much to manage at that time (this was before git/github) so it was merged into the same package. I am fine with splitting it back. Retaining the source history will be hard but most of the early histort with mayavi is anyway hard to find so I guess it does not matter. The tvtk stuff changes only with VTK releases but the mayavi code changes when there are core mayavi feature improvements.

prabhuramachandran commented 1 year ago

Merging this for now, I will try to cut a release tomorrow.

larsoner commented 1 year ago

Retaining the source history will be hard but most of the early histort with mayavi is anyway hard to find so I guess it does not matter.

One option would be to copy the entire repo to enthought/tvtk, and in enthought/mayavi do a rm -Rf tvtk and in enthought/tvtk o a rm -Rf mayavi. Then I think you keep the histories of each set of files. There would be some resulting unnecessary duplication across the two repositories in that case, but I don't think it's too bad. The git history of mayavi is not huge IIRC

prabhuramachandran commented 1 year ago

True, but this is one more headache, split the project, one more project to release and maintain and by a developer who already is not doing an ideal job of maintenance.

larsoner commented 1 year ago

Yes and a more complicated set of tests I suppose, too, because now mayavi at least needs to be tested against several tvtk versions as well. So maybe better left as a TODO for the distant future :)

prabhuramachandran commented 1 year ago

I have pushed a 4.8.1 release. I just realized that the release is stupidly including the tvtk_classes.zip but it is too late to fix it now so have added a ticket for it #1185 that I will look at later on -- before the next release. Anyway, 4.8.1 is now on pypi and hopefully should fix the issues people are facing. Thanks for all your help @larsoner.