pmorissette / ffn

ffn - a financial function library for Python

pmorissette.github.io/ffn

MIT License

1.94k stars 291 forks source link

Update ytd and mtd calculation #127

Closed sfjep closed 3 years ago

sfjep commented 3 years ago

Fixed bug in YTD and MTD calculation. YTD and MTD are not calculated for short return series, and are not necessarily equal to total_return.

MTD is only assigned something other than total_return, if more than one monthly return exists - which is unnecessary.

Same issue for YTD - but with a stricter condition of more data before it is calculated.

MTD and YTD returns are calculated after validating that a single daily return is available. If in the same month, the dp series can be used to find MTD, else the second to last value in the mp series is used. Same method for YTD calculation.

codecov-io commented 3 years ago

Codecov Report

Merging #127 (377972f) into master (171c46c) will decrease coverage by 0.07%. The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master     #127      +/-   ##
==========================================
- Coverage   62.31%   62.23%   -0.08%     
==========================================
  Files           4        4              
  Lines        1096     1099       +3     
==========================================
+ Hits          683      684       +1     
- Misses        413      415       +2

Impacted Files	Coverage Δ
ffn/core.py	`65.18% <66.66%> (-0.11%)`	:arrow_down:

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 171c46c...377972f. Read the comment docs.

sfjep commented 3 years ago

Does anyone see an issue with this solution?

Just to clarify. The current approach does not allow for calculation of MTD on a portfolio started intramonth. E.g. if a portfolio started trading on 01-03-2021, no MTD return would be available today.

Moreover, if a portfolio started trading on 01-12-2020 and YTD was calculated on 15-01-2021, YTD will be equal to the total return as the monthly return availability conditions are breached before the correct YTD is calculated.

timkpaine commented 3 years ago

@sfjep is it possible to add a test/tests for this?

sfjep commented 3 years ago

@sfjep is it possible to add a test/tests for this?

I added tests for the cases which I noted it currently fails. Let me know if any special cases are uncovered.

I'm uncertain about the build errors, anything to do about it on my end?

timkpaine commented 3 years ago

Yes this fails lint, you can check with make lint and attempt to autofix with make fix (or use the underlying flake8 and black commands)