Fix CAPEX shortcut silently pointing at the wrong column#34
Open
atalha1 wants to merge 1 commit into
Open
Conversation
CAPEX was getting silently redefined. names.py has: CAPEX = 'Capital Expenditures' ... CAPEX = CHG_FIX_ASSETS_INT = 'Change in Fixed Assets & Intangibles' since the file is written in alphabetical order by column name, the second line runs later and CAPEX ends up meaning 'Change in Fixed Assets & Intangibles' instead of 'Capital Expenditures', with no error, just wrong data for anyone using sf.CAPEX. root cause is in tools/generate_names.py: it dedupes shortcuts within a single column-name but never checks whether the same shortcut is attached to two different names on the server side. added that check, if a shortcut is claimed by more than one name it gets skipped for both (with a clear stderr warning) instead of one silently winning. figured skipping and warning is safer than guessing which name should keep it since I can't verify that against real data. also manually fixed the specific CAPEX line in the currently shipped names.py, since that part doesn't get regenerated until someone reruns the tool against the live server. fixes SimFin#10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10.
names.py currently has:
Since the file is generated in alphabetical order by column name, the second line runs after the first and
CAPEXends up meaning 'Change in Fixed Assets & Intangibles' instead of 'Capital Expenditures'. No error, no warning, just wrong data for anyone usingsf.CAPEX.Root cause is in
tools/generate_names.py. It dedupes shortcuts within a single column-name, but never checks whether the same shortcut got attached to two different names on the server side. Added that check: if a shortcut is claimed by more than one name, it's dropped from both (with a stderr warning listing which names collided) instead of letting whichever comes later alphabetically silently win.I skip and warn instead of guessing which name should keep the shortcut, since I don't have a way to check which literal string actually shows up in the real CSV column headers for that case, and getting it wrong would be worse than leaving it undefined.
Also patched the specific CAPEX line in the shipped names.py directly, since that file won't get regenerated until someone reruns the tool against live data.
Tested the generator fix locally against a fake JSON payload reproducing the CAPEX collision (monkeypatched
requests.get, no live server needed) and confirmed: the colliding shortcut gets dropped from both names with a warning, non-conflicting entries are written unchanged, and the multi-shortcut case (e.g.REVENUE = SALES = ...) still works normally.