-
-
Notifications
You must be signed in to change notification settings - Fork 326
feat: Compose groups via include-groups for reusable tabs and summaries #332
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,6 +159,41 @@ Watchlists and holdings can be grouped in `.ticker.yml` under the `groups` prope | |
| * If top level `watchlist` or `lots` properties are defined in the configuration file, the entries there will be added to a group named `default` which will always be shown first | ||
| * Ordering is defined by order in the configuration file | ||
| * The `holdings` property replaces `lots` under `groups` but serves the same purpose | ||
| * New: `include-groups` allows composing a group from other groups without duplicating entries. The effective watchlist is the concatenation of included groups’ watchlists (then the group’s own), de-duplicated by first occurrence; holdings are concatenated (lots aggregate by symbol automatically in summaries) | ||
|
|
||
| Example composite group: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be nice to include this in a collapsible secion since this likely may not be used by the majority of users: |
||
|
|
||
| ```yaml | ||
| groups: | ||
| - name: crypto | ||
| watchlist: [BTC-USD, ETH-USD] | ||
| holdings: | ||
| - symbol: BTC-USD | ||
| quantity: 0.75 | ||
| unit_cost: 35000 | ||
|
|
||
| - name: personal stocks | ||
| watchlist: [AAPL, MSFT] | ||
| holdings: | ||
| - symbol: AAPL | ||
| quantity: 20 | ||
| unit_cost: 130 | ||
|
|
||
| - name: personal holding | ||
| include-groups: [crypto, personal stocks] | ||
| # Optional extras specific to this composite | ||
| watchlist: [SOL.X] | ||
| holdings: | ||
| - symbol: MSFT | ||
| quantity: 3 | ||
| unit_cost: 310 | ||
| ``` | ||
|
|
||
| Notes: | ||
| - Includes are resolved recursively; cyclic includes are invalid. | ||
| - `default` (created from top-level `watchlist`/`lots`) may be referenced. | ||
| - Forward references are allowed: a composite group can include groups | ||
| declared later in the file. | ||
|
|
||
| ### Data Sources & Symbols | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -268,37 +268,44 @@ func getGroups(config c.Config, d c.Dependencies) ([]c.AssetGroup, error) { | |
|
|
||
| configAssetGroups = append(configAssetGroups, config.AssetGroup...) | ||
|
|
||
| for _, configAssetGroup := range configAssetGroups { | ||
|
|
||
| symbols := make(map[string]bool) | ||
| symbolsUnique := make(map[c.QuoteSource]c.AssetGroupSymbolsBySource) | ||
| var assetGroupSymbolsBySource []c.AssetGroupSymbolsBySource | ||
|
|
||
| for _, symbol := range configAssetGroup.Watchlist { | ||
| if !symbols[symbol] { | ||
| symbols[symbol] = true | ||
| symbolAndSource := getSymbolAndSource(symbol, tickerSymbolToSourceSymbol) | ||
| symbolsUnique = appendSymbol(symbolsUnique, symbolAndSource) | ||
| } | ||
| // Index groups by name for include resolution and validate duplicate names | ||
| groupsByName := make(map[string]c.ConfigAssetGroup) | ||
| for _, g := range configAssetGroups { | ||
| if g.Name == "" { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style nit: full word names |
||
| // unnamed groups are allowed unless referenced by include-groups | ||
| continue | ||
| } | ||
|
|
||
| for _, lot := range configAssetGroup.Holdings { | ||
| if !symbols[lot.Symbol] { | ||
| symbols[lot.Symbol] = true | ||
| symbolAndSource := getSymbolAndSource(lot.Symbol, tickerSymbolToSourceSymbol) | ||
| symbolsUnique = appendSymbol(symbolsUnique, symbolAndSource) | ||
| } | ||
| if _, exists := groupsByName[g.Name]; exists { | ||
| return nil, fmt.Errorf("invalid config: duplicate group name: %s", g.Name) | ||
| } | ||
| groupsByName[g.Name] = g | ||
| } | ||
|
|
||
| for _, symbolsBySource := range symbolsUnique { | ||
| assetGroupSymbolsBySource = append(assetGroupSymbolsBySource, symbolsBySource) | ||
| // Build final groups in declaration order using flattened config | ||
| for _, configAssetGroup := range configAssetGroups { | ||
| // compute effective watchlist/holdings | ||
| effWatchlist := configAssetGroup.Watchlist | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style nit: full word names |
||
| effHoldings := configAssetGroup.Holdings | ||
| if len(configAssetGroup.IncludeGroups) > 0 { | ||
| wl, hl, err := resolveGroupIncludes(groupsByName, configAssetGroup, map[string]bool{}) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| // de-duplicate watchlist preserving order | ||
| effWatchlist = dedupePreserveOrder(wl) | ||
| // lots are intentionally not de-duplicated here; aggregation is handled downstream | ||
| effHoldings = hl | ||
| } | ||
|
|
||
| assetGroupSymbolsBySource := symbolsBySource(effWatchlist, effHoldings, tickerSymbolToSourceSymbol) | ||
| groups = append(groups, c.AssetGroup{ | ||
| ConfigAssetGroup: configAssetGroup, | ||
| SymbolsBySource: assetGroupSymbolsBySource, | ||
| ConfigAssetGroup: c.ConfigAssetGroup{ | ||
| Name: configAssetGroup.Name, | ||
| Watchlist: effWatchlist, | ||
| Holdings: effHoldings, | ||
| IncludeGroups: configAssetGroup.IncludeGroups, | ||
| }, | ||
| SymbolsBySource: assetGroupSymbolsBySource, | ||
| }) | ||
|
|
||
| } | ||
|
|
||
| return groups, nil | ||
|
|
@@ -317,6 +324,82 @@ func getLogger(d c.Dependencies) (*log.Logger, error) { | |
| return log.New(logFile, "", log.LstdFlags), nil | ||
| } | ||
|
|
||
| // resolveGroupIncludes flattens include-groups recursively preserving order and detecting cycles | ||
| func resolveGroupIncludes(groupsByName map[string]c.ConfigAssetGroup, cur c.ConfigAssetGroup, visiting map[string]bool) ([]string, []c.Lot, error) { | ||
|
|
||
| wl := make([]string, 0) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style nit: full word names |
||
| hl := make([]c.Lot, 0) | ||
|
|
||
| if cur.Name != "" { | ||
| if visiting[cur.Name] { | ||
| return nil, nil, fmt.Errorf("invalid config: cyclic include-groups involving %s", cur.Name) | ||
| } | ||
| visiting[cur.Name] = true | ||
| defer delete(visiting, cur.Name) | ||
| } | ||
|
|
||
| for _, name := range cur.IncludeGroups { | ||
| inc, ok := groupsByName[name] | ||
| if !ok { | ||
| return nil, nil, fmt.Errorf("invalid config: include-groups references unknown group: %s", name) | ||
| } | ||
| iw, ih, err := resolveGroupIncludes(groupsByName, inc, visiting) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| wl = append(wl, iw...) | ||
| hl = append(hl, ih...) | ||
| } | ||
|
|
||
| wl = append(wl, cur.Watchlist...) | ||
| hl = append(hl, cur.Holdings...) | ||
|
|
||
| return wl, hl, nil | ||
| } | ||
|
|
||
| // dedupePreserveOrder removes duplicates from a list while preserving first-seen order | ||
| func dedupePreserveOrder(xs []string) []string { | ||
| seen := make(map[string]bool) | ||
| out := make([]string, 0, len(xs)) | ||
| for _, s := range xs { | ||
| if !seen[s] { | ||
| seen[s] = true | ||
| out = append(out, s) | ||
| } | ||
| } | ||
|
|
||
| return out | ||
| } | ||
|
|
||
| // symbolsBySource builds the list of symbols partitioned by quote source | ||
| func symbolsBySource(watchlist []string, holdings []c.Lot, tickerSymbolToSourceSymbol symbol.TickerSymbolToSourceSymbol) []c.AssetGroupSymbolsBySource { | ||
| symbols := make(map[string]bool) | ||
| symbolsUnique := make(map[c.QuoteSource]c.AssetGroupSymbolsBySource) | ||
|
|
||
| for _, sym := range watchlist { | ||
| if !symbols[sym] { | ||
| symbols[sym] = true | ||
| symSrc := getSymbolAndSource(sym, tickerSymbolToSourceSymbol) | ||
| symbolsUnique = appendSymbol(symbolsUnique, symSrc) | ||
| } | ||
| } | ||
|
|
||
| for _, lot := range holdings { | ||
| if !symbols[lot.Symbol] { | ||
| symbols[lot.Symbol] = true | ||
| symSrc := getSymbolAndSource(lot.Symbol, tickerSymbolToSourceSymbol) | ||
| symbolsUnique = appendSymbol(symbolsUnique, symSrc) | ||
| } | ||
| } | ||
|
|
||
| res := make([]c.AssetGroupSymbolsBySource, 0, len(symbolsUnique)) | ||
| for _, s := range symbolsUnique { | ||
| res = append(res, s) | ||
| } | ||
|
|
||
| return res | ||
| } | ||
|
|
||
| func getSymbolAndSource(symbol string, tickerSymbolToSourceSymbol symbol.TickerSymbolToSourceSymbol) symbolSource { | ||
|
|
||
| symbolUppercase := strings.ToUpper(symbol) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,9 +44,10 @@ type ConfigColorScheme struct { | |
| } | ||
|
|
||
| type ConfigAssetGroup struct { | ||
| Name string `yaml:"name"` | ||
| Watchlist []string `yaml:"watchlist"` | ||
| Holdings []Lot `yaml:"holdings"` | ||
| Name string `yaml:"name"` | ||
| Watchlist []string `yaml:"watchlist"` | ||
| Holdings []Lot `yaml:"holdings"` | ||
| IncludeGroups []string `yaml:"include-groups"` | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest changing the name to just |
||
| } | ||
|
|
||
| type AssetGroup struct { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed to call out this is new and it may not be new as time goes on.