- 
                Notifications
    
You must be signed in to change notification settings  - Fork 5
 
overhaul: add support for stock splits and automated awv reporting #6
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
Conversation
…ting share and forex queues, adapt transaction files to distinguish different events better, prepare transaction files for automated bundesbank reporting
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.
I checked the before-after comparison and they were equal. So far I couldn't check the foreign currency tab because of the account fill-up workaround
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.
Just one more round of automatic checks :)
| 
           #10 passes all tests now, let's get everything ready to merge! :)  | 
    
| 
           Great news, would you mind going over the conversations to see if there is still a comment not addressed? I'll do the same :)  | 
    
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.
I made some bugfixes and additions in my branch. They start at commit "Make "Anlage SO - Fremdwährungen" summary more precise".
Could you please look at them? After the fixes I think we can merge the two PRs.
…s, fix handling of schwab sell orders with potentially different prices per event, get split data from yfinance instead of manually curated list
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.
Newest version of YFinance is messy as it can be...
They removed session support, so you should remove it. However, they're adding it back in ranaroussi/yfinance#2505.
Then there are various other issues scattered around, like ranaroussi/yfinance#2474 which prevents stock splits retrieval, so this part of code wouldn't work anyhow.
It also complains about unset proxies, but it's just a warning: ranaroussi/yfinance#2512
I'll continue later.
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.
Fixed some minor issues on my side and adjusted better to the changed code.
Apart from these comments, I don't find anything else currently. I think the temporarily missing session handling can be solved by just removing the session parameter and as soon as it's back in YFinance, we can add it back here, too.
Everything else can be part of usual bug reports from wider audience :)
| 
           yfinance issues seems like a show-stopper for unfortunately  | 
    
…ache files, trying to use yf as little as possible
| 
           @szotsaki likely not nicely or over-engineered, but i revisited the historical prices through YF again 
  | 
    
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.
Thank you for the changes. I rebased #10, re-run the generator, and it worked the same as before :).
Let's merge!
(Ps. please merge my PR with the commit structure kept)
decimal.Decimalfor a more robust handling of floating point numbers