Closed rlskoeser closed 8 months ago
:exclamation: No coverage uploaded for pull request base (
develop@5ec72aa
). Click here to learn what that means. The diff coverage isn/a
.
@@ Coverage Diff @@
## develop #36 +/- ##
==========================================
Coverage ? 98.65%
==========================================
Files ? 4
Lines ? 223
Branches ? 0
==========================================
Hits ? 220
Misses ? 3
Partials ? 0
@ColeDCrawford I think you and I worked on the early stages of this months ago. Thought maybe you our @jdamerow could look over the more recent changes so we could get this merged.
I've added logic to track known granularity of the date with the degrees of precision that we support now, and updated string method and duration for Undate
objects with partially known dates. It occurs to me as I write this up that I didn't do anything about duration for UndateInterval
— but I'm not sure there's a meaningful way to calculate it, so maybe it's just not implemented.
You'll see there are some TODOs — I think there are decisions still to be made that will be easier to make once we add support for more formats.
I've added an example notebook comparing duration calculation in undate and in my Shakespeare and Company Project based on the exported values (durations are calculated by the code), as I had proposed doing in #57
This was a helpful exercise because I found and corrected an error in the logic for dates with unknown years that wrap from one year to the next.
I also found there's a difference in logic between the two implementations, which is whether you count both the start and end date or only one of them (or half of each). It seems ok to me to leave it as is for now, but might be something that should be configurable.
You can review the notebook in this branch.
update: I enabled reviewnb for this repository, so the notebook can be reviewed here: https://app.reviewnb.com/dh-tech/undate-python/pull/36/
@ColeDCrawford thank you for the review and comments! Glad the notebook was helpful. I think you're right, we have at least two new issues coming out of this: how to calculate duration, how to handle missing information. I'll create them and ask for input on slack, and then I'll go ahead and merge this.
ref #3 and #57