-
-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[💰20$ bounty] Inherited Tag Values are not Overwritten #1950
Comments
Looks like it should be easy enough to change by tinkering with the Also, would it be enough to do this just to postings, or might it be useful to also do this for accounts inheriting tags from parent accounts? |
Thanks for the discussion and PR, and sorry for the slow response. After looking into it.. yes, though docs don't explain it well, tags are multi-valued; they collect and add new values, rather than overriding a single value. This gets tricky, there are multiple cases to consider:
Currently in all of these cases we add rather than override values. This explains the behaviour you're seeing, and has the virtue of being simple and consistent. You expect it to override instead, in cases 2a and 2b ? This sounds reasonable, if it does not add confusion. I feel there might be ambiguities lurking, we should work out a complete set of tests. |
PS re this example:
The Loaned posting also inherits The pivoting makes things a little less clear, here are a few more views:
|
Ah, that explains it!
Yes, I expected child tag specifications to override parent specifications. For all my tags use cases, this is the only sensible behaviour I can think of.
This is complicated as there might be very many (unknown) values to exclude. |
I have clarified the current behaviour at https://hledger.org/dev/hledger.html#tag-values, for a start. |
Would it be an idea to be able to add something to the list of tags to enable this type of behaviour? Something similar to this special
Perhaps even having configurations for this at the top-level? I don't have enough time to scroll through documentation to see how this could be implemented exactly today, but it doesn't sound infeasible. |
The problem with the current behaviour is then that children can't remove a tag value, right? This would be |
I think the only viable way of introducing the override behaviour would be to introduce a {journal,file,transaction}-wide setting like @chrislemaire suggested. The current behaviour should remain the default because people might depend on it. |
I don't think options for this will be worth their cost - better if we can figure out and provide the best default behaviour, and if necessary document workarounds.. |
It's true that children can't remove tags inherited from parents, but maybe that's for the best ? If not, we should have some examples of real-world situations which need this functionality. |
Reasons against tag accumulation
Migration PathsSwitching to Tag OverwritingIf nobody actually uses the 'tag accumulation' feature, switching to 'tag overwriting' should be fully backwards-compatible. I can't think of a scenario where you would want actual tag accumulation. Especially as it's so hidden and not visible also with Introducing a New Tag-Setting SyntaxIf there is actual use of the tag accumulation behaviour, how about making adding, removing and resetting tag values an explicit operation:
But to be honest, I'd just switch to tag overwriting as a default behaviour. |
By the way, @chrislemaire's changes in #1955 produce the intuitive behaviour of overwriting tag values for me, thanks! |
Re that example: I see what you mean, but it's not very compelling because you could simply do:
I think I have described "adding/accumulating values" versus "overriding values" poorly, and in fact I'm confused about these now. And it's true that it's not easy to see the tags in effect on a particular posting (--pivot is an indirect way). So consider this example: here account, transaction and posting each have a uniquely-named tag and a same-named tag:
I used some debug code to see what's in each posting's
This shows that internally,
But to users, reports with tag queries behave as follows (at least,
Also, when pivoting on a tag with multiple values, this shows that... only the first (of
So I think the fine details of how tags behave can be specific to each command, but the general user-visible behaviour is as docs describe, currently:
|
Should |
I'd say |
I have another use case that would benefit from tag overwriting:
Without overwriting tags, the expense posting of the second transaction would have tags for both tax rates, which breaks the tax reporting. Using separate accounts for each tax rate is not viable as the tax rates are not relevant for budgeting, so the extra accounts (which would be needed for every expense account) would decrease the overview a lot. So I agree with @nobodyinperson's suggested migration path. I'd be fine with either:
|
Tags are
In the #1955 PR, tests for all of these would be good. Additionally, more real-world testing - there might be some consequences we haven't seen yet. I'll begin a mail list thread to clarify support for this. |
|
It looks to me like there is some consensus on overwriting tags as an option rather than the default should resolve this issue. I will take a look at implementing a solution and adding tests for some more complicated scenarios in the #1955 PR, taking into account the hierarchical ordering of Posting > Transaction > Account > Parent Account. If there is still debate over the exact syntax, I think it should be easy enough to change after implementing most of the PR, so I will start working on it now. |
I don't think the sample size is large enough; no-one on the list has responded yet. I am personally against adding syntax for multiple tag behaviours as I said. |
Ah I see, I'll hold off on it then until you have had response on the mailing list or until a different solution comes up. An issue I thought of with the syntax as well was that you might need to interpret tags left-to-right as well, adding some more cognitive depth to understanding tags. This is because situations like the following (with multiple values for the same tag assigned in a single comment):
which might be different from:
Unless I'm missing some already implemented nicer syntax for declaring multiple values for one tag, it matters here how tags are interpreted (left-to-right or right-to-left). |
These are things that would help this to move forward, I think:
These are things I generally want to avoid:
Currently I personally am mildly in favour of making value-overriding the default and only behaviour, after taking some more time to explore it, build confidence and gather support. |
Thank you very much for hledger, it is awesome! 🎉
Apparently there is a misleading behaviour when inheriting values of tags.
The docs (https://hledger.org/1.27/hledger.html#tags-1) mention that tags are inherited, but only mention value-less tags.
Consider this
book.hledger
:For consistency, higher-level tag values should overwrite tags values lower down the chain, right? At least a transaction/account shouldn't have two values of the same tag at once, right? But:
Conclusion
It seems that tag values are not overwritten down the chain but somehow still kept around. This can lead to confusion and inconsistencies in
register
for example.Proposal
I propose that tag values down the line should overwrite higher-level tag values.
The text was updated successfully, but these errors were encountered: