Skip to content

Move to toml - #105

Merged
csbrasnett merged 38 commits into
mainfrom
move-to-toml
Oct 2, 2026
Merged

csbrasnett merged 38 commits into
mainfrom
move-to-toml

Conversation

@csbrasnett

Copy link
Copy Markdown
Collaborator

move project to toml installation

can't work out how to solve the versioning atm, so hard coded for the moment.

@fgrunewald

Copy link
Copy Markdown
Collaborator

@csbrasnett thanks for making the effort and updating this one. Could you take a look at this PR https://github.com/gruenewald-lab/CGsmiles/pull/73/changes. We recently went trough the same for cgsmiles to make it align with vermouth and pysmiles.

Basically, I would prefer to keep pip over uv. The versioning has given me some pain as well, but the way it is setup in cgsmiles works.

@fgrunewald fgrunewald left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@csbrasnett nice job! I like the idea of moving the utility tools into the library and only exposing them from the package.

The only thing I dislike is the version handling, but it took me nearly a week to work that out for cgsmiles. Main points for dynamic versioning are: 1) add an empty line to the ymal as indicated; 2) switch the dynamic versioning on in the inint 3) possibly edit the deploy to checkout the version before deploying. If deploy fails once you changed 1+2, we can have a second look to see what needs to be changed exactly.

Comment thread fast_forward/__init__.py Outdated
Comment thread .vscode/settings.json Outdated
Comment thread LICENSE.txt
Comment thread pyproject.toml Outdated
Comment thread pyproject.toml
Comment thread pyproject.toml
Comment thread pyproject.toml Outdated
Comment thread pyproject.toml Outdated
Comment thread .readthedocs.yaml Outdated
@csbrasnett

csbrasnett commented Sep 14, 2026 •

Copy link
Copy Markdown
Collaborator Author

@fgrunewald thanks for the review, I probably should have marked this as draft - I intended to try and get my head round the dynamic versioning business today, thanks for the ready made solution!

Is there any more particular reason you favour pip over uv? My understanding is that uv pip ... should still be backwards compatible, while anyone who wants to can still use uv sync. That said, I'm not particularly bothered either way - I mostly switched to uv here to see how it could/should be done having not before!

Comment thread LICENSE.txt Outdated
@pckroon

pckroon commented Sep 14, 2026

Copy link
Copy Markdown

Is there any more particular reason you favour pip over uv?

We know pip. uv is too new and shiny and modern. Bah humbug

setuptools-scm also has a command line interface to help with version and file inclusion debugging. BUT that CLI also ignores any MANIFEST.in you use to exclude files from packaging

@csbrasnett

Copy link
Copy Markdown
Collaborator Author

Thanks both for reveiwing. @fgrunewald fyi I intend to address #91 and #94 asap too, might be easier to do so once this is merged? Will also try to find time to have a look at the state PRs that are still open.

@fgrunewald fgrunewald left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this should work now

@csbrasnett

Copy link
Copy Markdown
Collaborator Author

not sure what happened with the rebasing, mess probably inevitable from the refactor.

@pckroon had to remove the vermouth imports here because with exclude = ["vermouth.tests.*"] in the vermouth pyproject.toml now, can't import the itp comparison functions directly. Might be worth considering how to expose those into something importable?

@pckroon

pckroon commented Oct 2, 2026

Copy link
Copy Markdown

Oh that's reasonable. I'm open to either explicitely including the relevant helper files, or moving them out of the tests folder.

@csbrasnett

Copy link
Copy Markdown
Collaborator Author

@pckroon thanks, I'll merge this for now and have a think about what's best for the vermouth functions later

@csbrasnett
csbrasnett merged commit 80c5916 into main Oct 2, 2026
4 checks passed
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.

3 participants