Split percentages cannot exceed 100% of the transaction - allow to specify max allowed overshoot

[WolfgangFahl]

this is related to #62

After adding the split sum percentage to the err message i get:

pydantic.error_wrappers.ValidationError: 1 validation error for Transaction
splits
  Split percentages cannot exceed 100% of the transaction: 100.02 (type=value_error)

it would be good to be specify the allowed overshoot. Currently it's 0.0.1

[isaacharrisholt]

Good shout. In transactions with many splits, it's possible that multiple rounding errors could result in a greater overshoot

[WolfgangFahl]

see

class TransactionConfig:
    """
    transaction configuration settings
    """
    # maximum percentage overshoot of a split transaction above
    # 100%
    max_split_overshoot:float=0.01

which i change to 0.02 for my usecase which seems to work fine.

[patlachance]

The problem comes from the number of decimals for storing percentages, why keeping only 2 decimal positions when calculating the percentages? The odds of having rounding errors are statistically high when a transaction contains 10+ splits

From https://github.com/patlachance/quiffen/blob/main/quiffen/core/transaction.py#L183

        if total_percent - 100 > 0.01:

And from https://github.com/patlachance/quiffen/blob/main/quiffen/core/transaction.py#L488

                        round(split.amount / total * 100, 2)

I encountered the same problem and was able to resolve it by patching https://github.com/patlachance/quiffen/blob/main/quiffen/core/transaction.py#L488 to keep 9 decimal places. I should be ok with couple of hundreds splits, and shouldn't consume more memory!

                        round(split.amount / total * 100, 9)

[isaacharrisholt]

@patlachance This is a good solution. I assume this is only for the split.percent calculation?

I think ultimately a combination of both solutions will need to be used, but we need a good way of handling @WolfgangFahl's solution before implementing it.

@patlachance could you open a PR with this change? Ideally add a test with a large number of splits to check the fix works. There should be some other tests off the back of issues - please follow the docstring format used there.

[patlachance]

@patlachance This is a good solution. I assume this is only for the split.percent calculation?

I think ultimately a combination of both solutions will need to be used, but we need a good way of handling @WolfgangFahl's solution before implementing it.

Yes, my proposal was just to fix the split.percent calculation.

@patlachance could you open a PR with this change? Ideally add a test with a large number of splits to check the fix works. There should be some other tests off the back of issues - please follow the docstring format used there.

Sure, I will submit one soon!