Closed rlskoeser closed 2 weeks ago
@coderabbitai full review
[!IMPORTANT]
Review skipped
Auto reviews are disabled on base/target branches other than the default branch.
Please check the settings in the CodeRabbit UI or the
.coderabbit.yaml
file in this repository. To trigger a single review, invoke the@coderabbitai review
command.You can disable this status message by setting the
reviews.review_status
tofalse
in the CodeRabbit configuration file.
The changes encompass updates to several configuration files, workflow definitions, and documentation files for the undate
project. Key modifications include updates to the unit test workflow to support new Python versions, the introduction of a .pythonversion
file for version management, enhancements to the DEVELOPER_NOTES.md
for contributor guidance, and a comprehensive restructuring of the pyproject.toml
file to include project metadata and dependencies. Additionally, new classes for date handling have been introduced in the codebase.
File(s) | Change Summary |
---|---|
.github/workflows/unit_tests.yml |
Updated Python version to 3.12, expanded job matrix to include 3.13, modified cache path, and changed testing commands from tox to pytest . |
.pythonversion |
New file created specifying Python version as Python 3.12.7 . |
DEVELOPER_NOTES.md |
Added detailed instructions for setup, git flow, virtual environment, testing, and documentation building. |
pyproject.toml |
Restructured to use hatchling , added project metadata, dependencies, optional dependencies, and URLs. |
src/undate/date.py |
Introduced Timedelta and Date classes for enhanced date handling, along with a DatePrecision enum. |
src/undate/dateformat/base.py |
Replaced lru_cache with cache for the import_formatters method in BaseDateFormat . |
.github/workflows/unit_tests.yml
file in PR #88, which also involves modifications to the testing framework and caching mechanisms.🐰 In the garden of code, we hop and play,
With new contributors joining the fray.
Workflows updated, and docs refined,
A brighter future for all we find!
So let us celebrate, with joy and cheer,
For every change brings us closer, my dear! 🌼
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Observations on coderabbit
(will keep updating):
ruff
even though we're using black
/ flake8
. ruff
is definitely worth moving too but worth noting that it didn't align with the project's conventions. I think this is going to be one of the negative externalities of gen AI code tools - they will work well on high profile languages / frameworks with lots of existing examples and much less well on those outside of that scope. In this case if you want to use a different formatter or linter that doesn't have support in their agent, then you are SOL.I'll update this and reconcile with the other merged PRs, address the flagged comments about type hints, and then do some testing and documentation of benchmarking as suggested before I merge this.
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
@ColeDCrawford I was hoping I could just clean this PR up and merge without further review, but an error in the S&co example notebook caused me to find some problems with how I was handling duration logic in the new numpy-based implementation, which resulted in a bit more substantial change (and effort!) than I'd anticipated. I'm going to tag you for review on the updates.
I've resolved all the coderabbitai comments that I've either addressed or don't care about; one interesting/tricky one left open for now. It flagged a lot of things, which I have mixed feelings about - glad to have things cleaner and in better shape, but it felt like it resulted in more work for me.
FWIW I also went ahead and switched from black to ruff in the pre-commit hook. It doesn't seem to be correcting unused imports automatically, so if you have a set of options you prefer maybe you could open a PR to add them after this is merged.
ref #77
datetime.date
previous notes
I implemented this as spike to see how hard it would be to use numpy's datetime64 instead of python's builtin datetime.date. Using
np.datetime64
gets us way past the year limitation ofdatetime.date
([2.5e16 BC, 2.5e16 AD] for day-precision dates).We can make this an optional dependency so you would only install numpy if you need the expanded year range. Making it optional makes the code and the testing more complicated. Do you think numpy as a dependency would be a concern for folks doing e.g web-based projects?
(type check is failing because I haven't bothered updating the types yet)
Summary by CodeRabbit
New Features
DEVELOPER_NOTES.md
file providing comprehensive contribution guidelines and setup instructions.Timedelta
andDate
classes for enhanced date and time handling.Documentation
DEVELOPER_NOTES.md
with detailed setup and testing instructions..pythonversion
file for specifying the Python version.Chores
hatchling
as the build system and updated dependency management.