Add requirements.txt and update readme in wheel

[matt-o-how]

Does what it says on the tin

[socket-security]

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
pypi/chia-blockchain@2.3.0rc4	environment, eval, filesystem, network, shell, unsafe	0	14.7 MB	chia-network
pypi/iniconfig@2.0.0	filesystem	0	15.4 kB	hpk, ronny
pypi/maturin@1.5.1	environment, filesystem, shell	0	41.3 MB	konstin, messense
pypi/packaging@24.0	environment, eval, filesystem, shell	0	2.4 MB	brettcannon, dstufft, pf_moore, ...1 more
pypi/pluggy@1.5.0	environment, filesystem, network, shell	0	232 kB	The_Compiler, flub, goodboy, ...3 more
pypi/pytest@8.2.0	environment, eval, filesystem, network, shell, unsafe	0	5.7 MB	The_Compiler, anatoly, flub, ...4 more

View full report↗︎

[coveralls-official]

Pull Request Test Coverage Report for Build 8894841227

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 71 unchanged lines in 11 files lost coverage.
• Overall coverage decreased (-0.3%) to 84.832%

---

Files with Coverage Reduction	New Missed Lines	%
crates/chia-protocol/src/bytes.rs	2	87.23%
crates/chia-protocol/src/chia_protocol.rs	2	0.0%
crates/chia-consensus/src/gen/owned_conditions.rs	3	91.3%
crates/chia-consensus/src/error.rs	3	0.0%
crates/chia-bls/src/signature.rs	4	95.97%
crates/chia-bls/src/public_key.rs	4	92.94%
crates/chia-bls/src/secret_key.rs	5	94.74%
wheel/src/run_generator.rs	7	71.79%
crates/chia-consensus/src/fast_forward.rs	8	97.98%
wheel/src/api.rs	15	77.09%

Totals
Change from base Build 8807260736:	-0.3%
Covered Lines:	11756
Relevant Lines:	13858

---

💛 - Coveralls

[Rigidity]

When I pip freeze with this locally, I get a lot more dependencies - should we be including those in this file too, or no? I don't know much about Python dependency management

[matt-o-how]

I believe those are installed from these and that doing it this way captures those too.
Although, I also don't know much about Python dependency management so I may be wrong.

[richardkiss]

You can think of dependency management as applying additional constraints to a system.

Compare to sudoku. You have a bunch of empty squares (the version numbers for each dependency) and a bunch of numbers that can go in those squares (in this case, the list of all versions for a particular package). Then you have to find a set of numbers that go in those squares that satisfy the constraints. A general form of this is known as a "SAT solver" https://en.wikipedia.org/wiki/SAT_solver and there is a lot of academic work in trying to get SAT solvers to be as efficient as possible because they can be applied to a lot of sudoku-like search problems.

(The comparison becomes a lot more obvious when you use >= constraints rather than == since == is a lot more restrictive and plain-spoken about its intention.)

[richardkiss]

That said, there are two points I want to make:

1. I believe there's a push towards putting dependency constraints in pyproject.toml rather than requirements.txt. See for example https://github.com/Chia-Network/hsms/blob/main/pyproject.toml#L11
2. maturin is a build-time dependency only. It does not need to be present when you're using the compiled wheel. So its version should really go in pyproject.toml (which is the "this is how to build me" file); see https://github.com/Chia-Network/chia_rs/blob/main/wheel/pyproject.toml#L2

[richardkiss]

Consider instead

1. python -m pip install -e . (for a "development install" which points to the python files in the repo rather than making a pristine own copy that can let you "try out" python changes without reinstalling) or
2. python -m pip install . (to make a pristine own copy that ignore further repo changes).

[richardkiss]

Now, when you're doing development, you probably do want maturin in your local venv so you can test rust changes by doing maturin develop rather than pip install -e . again, which takes longer because it creates a sandboxed venv to do the build.

So to do development, I guess ideally what you want

cd wheel
python -m venv venv
. ./venv/bin/activate
python -m pip install maturin==(whatever the version listed in pyproject.toml is)
python -m pip install -e '.[dev]'
maturin develop
python -m pytest ../tests

The .[dev] line means "please install additional package that this pyproject.toml file declares as being useful for development", so stuff like pytest, ruff, mypy, etc. We have to add stuff like in https://github.com/Chia-Network/hsms/blob/main/pyproject.toml#L19

Also, for obvious reasons, I'm not happy with the python -m pip install maturin==(whatever the version listed in pyproject.toml is) line. We could add maturin to the dev = ["maturin==...] line, but then we would have it in two places (the build section and the dev section) which is a recipe for disaster if they get out of sync and some incompatibility pops up and it works locally because we're using the dev version of maturin but not in CI because it uses the build version. I expect there is some pip thing to install the build tools, but I don't know what it is.

This comment is very dense with information, so feel free to ask for clarification if I'm not being clear.

[matt-o-how]

the python dependencies are all '.[dev]' dependencies as the python is only used for running pytests

[richardkiss]

There is a pyproject.toml file so there is a “python” project… it’s the wheel chia_rs. It happens to not have any python source code at the moment, but that’s okay. The CI still uses the pyproject.toml file to build it (or at least, it should!). The idea of pyproject.toml is to standardize how python-targeted wheels build lifecycles are specified, so IMO it's reasonable to add the dev dependencies here even if most developers end up using the maturin develop shortcut during their build/test development cycle.

[richardkiss]

I guess you're talking about https://github.com/Chia-Network/chia_rs/blob/main/README.md#python-tests ?

Looks good. It might be helpful to note in the README that this mechanism to run the tests is a hack that is necessary because the chia_rs and chia_blockchain wheels currently have a cyclic dependency. At some point this can be fixed and then we'll be able to run the chia_rs tests with pip install .[tests] and pyproject.toml like I was hinting at above.

In any case, I think we can close this PR without merging it now. Thanks to @matt-o-how for bringing up this discussion. Prior to this, I hadn't internalized the cyclic dependency with chia-blockchain.

[arvidn]

I think richard's comment about pyproject.toml should be a blocker

[richardkiss]

@arvid I discussed this further with Matt and his main goal here is to provide some instructions on how to run the tests. Knowing how to do this is handy for new people who are working on this project, and it's a bit tricky because it requires chia-blockchain, which tries to pull in its own preferred version of chia_rs.

In other words, there is a cyclic dependency between chia_rs tests and chia_blockchain. So this pyproject.toml pattern doesn't work correctly (pip install '.[dev]' complains that the version of chia_rs pinned is wrong for the version of chia-blockchain). So we need another clunky

@arvid You must have some way to set up to run the tests. Can you describe what that is? And we can put that in the README for now, maybe along with a disclaimer as to why we have to do it this way for now.

If we factor out a base layer from chia-blockchain that chia_rs can depend upon without pulling in the rest of chia-blockchain (ie. fix the cyclic dependency), we can move to more normal way of doing this.

[Rigidity]

We're already working on untangling the circular dependency - moving sized ints/bytes types to chia_rs has been the first step, and I'll continue work on this in the next few weeks

[richardkiss]

We're already working on untangling the circular dependency - moving sized ints/bytes types to chia_rs has been the first step, and I'll continue work on this in the next few weeks

That's good, but for now, it still might be good to have a README that describes how to run the tests, no?

[Rigidity]

That's good, but for now, it still might be good to have a README that describes how to run the tests, no?

Yeah, I can help write this up this week if you want.

I always just run pytest tests but there are a couple dependencies I suppose.

[Rigidity]

The readme should be updated @richardkiss - let me know if this covers it for now or if more details are needed

outdated