Closes #3: add unit tests for PeriodAxis #4
Conversation
| period_axis = PeriodAxis.from_series(series_date) | ||
| assert period_axis.between(np.datetime64("2023-01-02"), np.datetime64("2023-01-03")).tolist() == [1] | ||
| assert period_axis.between(np.datetime64("2023-01-02"), np.datetime64("2023-01-03"), closed=Closed.BOTH).tolist() == [1, 2] | ||
| assert period_axis.between(np.datetime64("2023-01-02"), np.datetime64("2023-01-03"), closed=Closed.RIGHT).tolist() == [2] No newline at end of file |
There was a problem hiding this comment.
also we need general linting and formatting like last time :)
There was a problem hiding this comment.
Thanks. I added a pre-commit dependency and defined formatting and type check as the steps in the pre-commit. I'd love to show you but I currently can't commit because it is failing type checks lol. But let me know if you think if it's a good idea, once you get back to me on the closed parameter issue, and I push the changes.
There was a problem hiding this comment.
i've seen some integrated CIs running in GitHub actions that gets auto run and tagged onto the commit/PR. that issue you're dealing with is the exact reason i'd be hesitant to add a pre-commit hook
There was a problem hiding this comment.
Hi James, please have a look at the pre-commit yaml file, let me know if you think the steps are okay to include.
There was a problem hiding this comment.
it looks like exactly what we should run as pre-commit, check out the checks i added for #7. i'll merge these pre-commit changes along with that.
i'll just need you to 1. create a new branch with only the pre-commit changes, 2. amend the pre-commit changes off of this commit, and then i can incorporate that commit into #7.
There was a problem hiding this comment.
Done. The branch I created is called: add-pre-commit-steps, but it only has pre-commit and github CI actions. Let me know if I misunderstood you.
There was a problem hiding this comment.
thanks! that's all, it's merged now
xiaotongchang-creator
force-pushed
the
unit-test-period-axis
branch
4 times, most recently
from
5325d35 to
7954ad2
Compare
xiaotongchang-creator
force-pushed
the
unit-test-period-axis
branch
from
7954ad2 to
6963f22
Compare
|
nice! this brings us up to about 40% coverage overall already |
2 participants
No description provided.