Contributing Dietz method
francocalvo opened this issue · 10 comments
Hey!
I've been playing with the code for a while.
First, I packaged it with Poetry and Nix. It should be easy to release it to PyPi.
Secondly, using most of the logic from the current code, I changed the compute_irr for compute_dietz. There are some changes around the code on how the truncate works, but otherwise it's pretty much unchanged.
Would you be interested in a PR for any of those things? It would be cool to have both the time and money weighted performance. It would also allow to add benchmarking.
Sure, sounds interesting.
Perfect!
This is what it would look like more or less:
Do you have a test beancount file to use?
Also, I've done these three things. Please let me know if you'd merge them:
- Created a pyproject.toml using Poetry for dependency management, building and tool configuration (pyright, ruff).
- Created a 'nix flake'. It would be useful to package it easily to add it to pkg managers like apt.
- Fixed all important alerts for those tools above, and fixed some type hints that weren't correct. This is just for the
reports.pyfile. I didn't change any actual logic (except for the demo above, but I can revert that).
The last point comes with many changes to the structure of the file, like replacing os calls with pathlib for example.
I don't have a test Beancount to use. We should probably generate one programmatically.
- SGTM
- SGTM (if you'll do it, I have zero time for packaging)
- SGTM
Excellent. I'll try to get the PR ready today.
Regarding the usage of Poetry, I'd like to ping @andreasgerstmayr first. It would overwrite your pyproject.toml. Is this OK for you too? I know you use it in a couple of plugins
Regarding the usage of Poetry, I'd like to ping @andreasgerstmayr first. It would overwrite your
pyproject.toml. Is this OK for you too? I know you use it in a couple of plugins
Sure, as long as it's still a valid Python package which can be defined as a dependency of other projects :)
Everything sounds good to me, except the conversion to Poetry. I don't like that it is not designed to compose with the rest of the Python packaging ecosystem, and I don't have time to learn a new way of doing things. What does Poetry bring to the project that cannot be implemented with other more standard tools?
@andreasgerstmayr good to hear that. It should behave exactly like other packages.
@dnicolodi how is it not designed to compose with the rest of the Python packaging system? You can still pip install it or use it as a dependency in a setup.cfg.
For example, you should be able to install it like:
pip install git+https://github.com/francocalvo/beangrow.git@modified-dietz-returns
PS:
What it brings, In my eyes, is faster builds, a lock file to ensure reproducibility, it respects PEP 518 and PEP 621 better than having a separate setup file (imo). It also creates venv using the lock file.
PPS:
I meant to share this one the last comment about the PEP621: Poetry PR 9135.
If installing the package is the only thing you want to do, you can do it without modifying the current pyproject.toml. Poetry is also a dependency manager and a virtual environment manager, that requires to use a non-standard definition of the package in pyproject.toml. I don't see why you need to force your use of development tools on everyone.
Sorry, I edited the comment almost at the same time you commented.
It's not just for installing the package. The point of that was that it shouldn't affect the user.
I'm not trying to force anything on anyone, I'm doing a PR and asked about this specifically. I'm open to change it, and I have it isolated in a commit so I could revert it.
Poetry is the tool I use at work and in personal projects. It has been a very valuable tool for me in deploying Python packages.
Hey there! Just wanted to share a bit. I intend to work a bit more on this, and publish it if possible. I just changed jobs so life got busy. I'll get to it soon tho. Cheers :)