re.match(a, (b or c)) does not work

[behinger]

I have a problem with this line:

beancount-categorizer/beancount_categorizer/categorizer.py

Line 21 in 9900eaa

if re.match(payee, (entry.payee or entry.narration)):

I don't think it works the way you intended it. Not a python pro, but in my small test, python just ignores the parantheses.

import re
a = r"^\d"
b = "nope"
c = "1"
print(re.match(a,(b or c)))

This returns None, whereas I expected it to return True

I replaced the or with a + because I want to match both anyway :)

[bratekarate]

Hm, I think it works as I originally intended. If entry.payee is none, then entry.narration is checked for a match instead.

Why would you want your statement to return True? Can you tell me more about your use case?

Edit: I think I may start understanding your issue. I would have to look further into it and remember what my initial intended behaviour actually was.

[behinger]

ah, maybe I misunderstood - I thought it was supposed to check both payee and narration and return a match in any of them.

I have quite a lot of transactions and I need to check both, preferably at the same time. So I modified this code as described above. For my use case, this solution works fine.

[bratekarate]

I checked my README, the code and the beancount documentation again more thoroughly to remember what my intention was. In fact, payee is optional in beancount. If only one string is provided, beancount will make it the narration.

Originally I only checked the payee and thought it is sufficient, being a ledger-cli user. But when some transaction did not receive a payee field by the categorizer, I muted the exception by adding this or clause.

Actually it may not be a bad idea to check in both fields. The results may be more consistent, or at least less surprising to users. Do you want to make a PR, so I don't have to code the same thing again? I'm actually pretty rusty in python too and would have to read the regex documentation of python again :)

Preferably, I would like to have the option to turn this behaviour on or off via config, but I don't think anyone minds if we just switch to this behavior for now (I presume the user base is rather tiny).