Skip to content

Upgrade to pyproject.toml#78

Open
CharlieAtSchneider wants to merge 13 commits intoschneider-electric:masterfrom
CharlieAtSchneider:pyproject
Open

Upgrade to pyproject.toml#78
CharlieAtSchneider wants to merge 13 commits intoschneider-electric:masterfrom
CharlieAtSchneider:pyproject

Conversation

@CharlieAtSchneider
Copy link

No description provided.

@CharlieAtSchneider
Copy link
Author

This is the first step in a series of PRs. In the current state, it doesn't work properly. The next PR will address that

@kimgr
Copy link
Contributor

kimgr commented Sep 24, 2025

There's still way too much going on at once for my taste.

  • Copyright years
  • README rename
  • README removal of dependency information
  • All source code moved to ./src
  • Add Ruff dependency
  • Add pyproject.toml
  • Update pyparsing dependency from >2.0.0 to >=3.2.5
  • Add unnecessary pyasn1 dependency for 0.6.1. Note that pyasn1 is not required at runtime, only for testing. We generate pyasn1 source code, but have no dependency on the library.
  • Add hard dependency on uv -- I would prefer for the library to be git-clonable and runnable directly without any system dependencies. I don't know anything about uv, so maybe I'm misunderstanding? Might be nice to include a little section in the README on how to clone and run tests. If that requires installing system packages, I'm mildly opposed.

Most of these things are very useful, even though I disagree with some (perhaps misguided!). It's much easier to make progress with the non-controversial stuff if you keep them separate from the bigger changes.

In the current state, it doesn't work properly. The next PR will address that

This is also a bit of a red flag -- smaller changes should make it easier to keep mainline working at all times.

@CharlieAtSchneider
Copy link
Author

CharlieAtSchneider commented Sep 25, 2025

Alright, I removed some of the changes.

However, the transition between build systems inherently requires a lot of changes. I don't think I can reduce it any further for one logical commit.

The dependency to uv is something we have used internally at Schneider for a while now. It simplifies a lot of things when working in the Python ecosystem:

  • Automatically installs the required Python version if not installed, and makes it possible to run more reproducably
  • Automatically setups and uses .venv
  • It's really, really fast
  • It has gotten rather popular, even overtaking Poetry in downloads from Pypi, so it's not a niche tool, and it's quickly gaining momentum based what I can see on forum posts online
  • It has a lock file, fixing Python's problem of non-reproducible builds. Together with the capability of locally installing Python versions, the Python project works on every machine with any OS. Very similar to how Cargo for Rust works.

And you don't really need to use uv. pyproject.toml is standardized, and only uv_build (replacing setup.py) makes use of uv.

I think it's rather elegant. Simply one command to run everything, similar to something like Cargo for Rust. We use it a lot at Schneider to speed up our CI too. (lot's of slow pip commands quickly adds up)

Regarding the hard dependency on new dependencies, that's just me being used to packaging Rust, where newer dependencies always are better, unlike Python, where that can hinder usability for others using the library.

Regarding the red flag - I never got pyas1ate to work. It crashes whenever I run ./basic_test.sh. As it currently stands, it's not compatible with some of the changes in Pyparsing. A lockfile would fix this :) But I tried to honor the request of not changing too much in one PR (which is very reasonable) so I'll focus on that in future PRs

@CharlieAtSchneider
Copy link
Author

Also, good thing you mentioned pyasn1 is only a dev dependency - now that is reflected in pyproject.toml too!

@CharlieAtSchneider
Copy link
Author

I could remove uv pretty easily, but I would need to change the build system to something like hatch for that to work.

@CharlieAtSchneider
Copy link
Author

CharlieAtSchneider commented Sep 25, 2025

uv is now removed as a development dependency. As a result, the project currently lacks any kind of build system. The next PR will fix that

@CharlieAtSchneider CharlieAtSchneider changed the title Use pyproject.toml and move to uv Upgrade to pyproject.toml Sep 25, 2025
@mickaelgouet-se
Copy link

Please update the LICENSE.txt file to a BSD-3 standard license. Thx

@kimgr
Copy link
Contributor

kimgr commented Sep 25, 2025

It has a lock file, fixing Python's problem of non-reproducible builds. Together with the capability of locally
installing Python versions, the Python project works on every machine with any OS. Very similar to how
Cargo for Rust works.

I guess I've never had that problem. I've used virtualenv or python3 -m venv + pip install -e . for ~15 years to get a development sandbox up. With correct package dependencies in setup.py, that should just work.

I don't want to stand in the way of improvement, especially since I no longer use asn1ate. But don't underestimate how reluctant old codgers like myself are to install system packages to experiment with a freestanding Python package :-)

@CharlieAtSchneider
Copy link
Author

@mickaelgouet-se I think it's already correctly licensed? Am I missing something? That text looks like a BSD license to me

@CharlieAtSchneider
Copy link
Author

CharlieAtSchneider commented Sep 26, 2025

Alright, I think the last set of commits added some compromises that makes everyone happy!

Now there are instructions on how to test and build the project. (tests still fails though, but not because of any changes I have made)

@mickaelgouet-se
Copy link

@mickaelgouet-se I think it's already correctly licensed? Am I missing something? That text looks like a BSD license to me

@nyameen-se can you please help? I believe you raised this concern

@nyameen-se
Copy link

@mickaelgouet-se I think it's already correctly licensed? Am I missing something? That text looks like a BSD license to me

@nyameen-se can you please help? I believe you raised this concern

Right, I was expecting to see BSD 3 Clause in the license file, but according to the text, it is BSD 3 Clause. This example repo has the first line of the LICENSE file with BSD 3-Clause License to make it evident. Sorry for the confusion, I don't think anything needs to be done... unless you really want to add BSD 3-Clause.

Copy link

@mickaelgouet-se mickaelgouet-se left a comment

Choose a reason for hiding this comment

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

All good for me

@kimgr
Copy link
Contributor

kimgr commented Sep 26, 2025

I'm going to drop out of review, I don't seem to be getting my point across about more focused patches. Enjoy!

Please do keep me in the loop for functional changes to the parser/code generator, I might remember something relevant there.

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