chore: add nanobind to build system#85
Conversation
|
@sbillinge PR ready for review. Since nanobind doesn't provide a conda library we have to build it our own, the build_ext_with_nanobind function builds nanobind with its recommanded arg (this is a bit ugly but seems to be the only way until we introduce cmake). |
|
@SYMSCAE on a separate PR please could you do the upgrade of the pre-commit packages so that everything is at least passing pre-commit? For the failing tests I don't really have a clear idea abot which ones we expect to be falling (didn't migrate those things) and which are concerning (something broken). Maybe it is best of you can add a few words in the comments about these issues and what you want me to do, for example, to merge an intermediate step, or not merge because we want to get all tests passing on the branch, for example.. |
Sure I'll make another pr to upgrade pre-commit to main and see what that happens. The tests are failing because ci is fetching libdiffpy from conda so it will continue to fail until we make a new conda release. It can be fixed if we change the workflow to fetch and build libdiffpy from source but I guess that's not preferred? |
I see. Is libdiffpy ready to release? Is it passing tests? |
It is passing all tests on my end, but we might need to set version requirements for diffpy.srreal since this is breaking abi. Maybe we need a new version tag? |
I think we just need a new release. @ycexiao or Rundong can help with that. |
Hi @sbillinge @SYMSCAE, I am happy to help. Just to clarify, do you mean a new release of libdiffpy? Are there any other compatibility issues I need to pay attention to? |
yes. Libdiffpy has been changed to be compatible with nanobind. We need to think a bit about this because this may not be backwards compatible so we need to make sure that everything works. If someone installs a version of srreal that uses boost then it needs to load the old version of libdiffpy but if it loads a version of srreal that uses nanobind, it loads the new version. Please could we look into how to handle that elegantly? |
Hi @ycexiao, yes it is a new release of libdiffpy. But before that, @sbillinge should we first merge changes to main for libdiffpy? Or you would like to have tests wired in the ci first? |
indeed, this will be part of the release sequence. |
chore: updated pre-commit hook dependencies
|
@sbillinge I've synced changes with main, we should get rest of the ci working once the new version of libdiffpy is available. |
@sbillinge there is a closely related FAQ in the In summary:
Also I think we should update the Pease see if it is what we want. |
This PR updates cpp language version to c++23, replaces boost smart pointer usages with stl implementations and adds nanobind to the current build system.