FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

refactor: migrate Boost.Python syntax to nanobind by SYMSCAE · Pull Request #86 · diffpy/diffpy.srreal · GitHub

Repository navigation

refactor: migrate Boost.Python syntax to nanobind - #86

Closed
SYMSCAE wants to merge 25 commits into
diffpy:migration-nanofrom
SYMSCAE:nb-syntax
Closed

SYMSCAE wants to merge 25 commits into
diffpy:migration-nanofrom
SYMSCAE:nb-syntax

Conversation

SYMSCAE commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

This PR is the initial port that migrates the original Boost.Python bindings to nanobind as mentioned in diffpy/pyobjcryst#88, noticed future improvements will be mentioned in TODOs but will be addressed in future PRs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is a very early port with only syntax checks performed. The pickle suite naming is left but underneath it is handled differently, the final ways of pickling depends on migration for rest of the files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There's an explicit behavior change (or enhancement?) in atomadapter_indexing that non-unit and negative-step slices are handled. We may need to test this on all of the packages to see if it breaks anything.

SYMSCAE commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@sbillinge Now the syntax have been migrated and build and tests are passed. I'll keep marking the PR as draft since I found some of parts are not done elegantly, but we can start reviewing this.

Copy link
Copy Markdown
Contributor

@SYMSCAE pleae can you, on a separate PR, do the updates of the pre-commit versions so we can get that passing also?

I will start to look at this. @pavoljuhas if you have time, there is more c++ reviewing here. @ycexiao @danielsirakov, if you guys have time, please could you also take a look. This is the work to migrate from boost to nanobind

SYMSCAE commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@SYMSCAE pleae can you, on a separate PR, do the updates of the pre-commit versions so we can get that passing also?

I will start to look at this. @pavoljuhas if you have time, there is more c++ reviewing here. @ycexiao @danielsirakov, if you guys have time, please could you also take a look. This is the work to migrate from boost to nanobind

I'm working on that, but the change of docformatter breaks some things. I'll open a new PR and bring the discussion over there.

Copy link
Copy Markdown
Contributor

I merged that PR so you can update main and merge it into this branch

Copy link
Copy Markdown
Contributor

@SYMSCAE this PR seems to have force-pushes on it which we don't want. I wonder how much work it would be to do this over on a clean PR without the force pushes. As a general matter, please never force-push. This is bypassing all of git's protections which is dangerous and it usually means that some incorrect workflow is being pushed, again, with often unforeseeable downstream consequences.

I know there is quite a bit of work here so let's discuss it, but I would be much more comfortable having the work on a clean and properly sequenced (pre-commit fixes ahead of code fixes for example) set of PRs.

SYMSCAE commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@SYMSCAE this PR seems to have force-pushes on it which we don't want. I wonder how much work it would be to do this over on a clean PR without the force pushes. As a general matter, please never force-push. This is bypassing all of git's protections which is dangerous and it usually means that some incorrect workflow is being pushed, again, with often unforeseeable downstream consequences.

I know there is quite a bit of work here so let's discuss it, but I would be much more comfortable having the work on a clean and properly sequenced (pre-commit fixes ahead of code fixes for example) set of PRs.

Sure, I can open a new pr and cleanup the commits. Could you please sync the migration-nano branch with main.

SYMSCAE commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

PR closed due to #89 being merged.

SYMSCAE closed this Oct 8, 2026
SYMSCAE deleted the nb-syntax branch October 8, 2026 22:01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

2 participants


Back | FazBrowse Home | New Git URL