Closed nicmendoza closed 4 years ago
@adamjstewart is it possible re-running the build could resolve this issue? If I understand the logs, it seems like there's a plugin version resolution conflict with pytest specifically on Python 3.5 but the build+test are succeeding otherwise.
Added a minor commit to re-trigger the build ☝
Merging #11 into master will not change coverage. The diff coverage is
100.00%
.
@@ Coverage Diff @@
## master #11 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 266 269 +3
=========================================
+ Hits 266 269 +3
Impacted Files | Coverage Δ | |
---|---|---|
fiscalyear.py | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 365f86e...faa3711. Read the comment docs.
Had to pin pytest and pytest-mock to old versions to maintain support for python 2 .7 (assuming that's still a requirement)
Hi @nicmendoza, thanks for the contribution!
First impression: I like it, and I think the way you have things implemented looks correct. I'll go over this in more detail over the next couple of days.
With regards to the pytest issue: I think pinning the version is the right thing to do. We don't use any new features of PyTest anyway. I would like to continue to offer Python 2 support because a lot of people are stuck with Python 2, especially in industry where they can't update their own computers. I use a couple of supercomputing clusters that are still stuck on Python 2.6 unfortunately. I've thought about dropping support for Python 2, 3.0-3.4 so that I can add type hints, but so far there hasn't been any demand for it.
This PR specifically adds a fiscal_month attribute (and does not add a correspondingFiscalMonth object). I can follow up with fiscal_week and fiscal_day if desired, but I really needed this functionality for my use case and I'm hoping this PR stands on its own. It's pretty simple, and I think the approach could be quickly adapted to those two additional calculations if/when desired.
Please let me know if the PR is missing anything.