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
sage.finance - add expand and refine finance.Stock #3621
Comments
Replying to @cswiercz:
This ticket depends on #3356. |
comment:5
add_load_file1.patch - Split up .historical into three parts. Added a .load_file() function that will load data from a specified file if the file formatted correctly. |
comment:6
.load_file() does not work for large files due to python's .read() size limitations. Next patch will read the file by line to get around this. |
comment:8
REFEREE REPORT: Great work! Now be professionals and fix everything below :-)
must become
We actually do test all optional doctests sometimes, and do expect them to pass on a reasonably configured machine. I suggest the following workaround. Make a subdirectory of SAGE_ROOT/examples called "finance". Put your file in there. Do hg ci in there to add it to the examples repo. Post a patch using "hg export tip" in that repo. If you get stuck, let me know. Then in the doctest just put SAGE_ROOT + 'examples/finance/your_filename.txt'. And you won't have to make it optional, which is always a plus.
You mean to write:
A doctest should illustrate this exception maybe, so you would have caught this bug.
|
comment:11
Faulty formatting with part5.patch? Following error message when applying the patch.
|
comment:12
Patch 6 is a duplicate of patch 5. Part 6 is supposed to add a new doctest. I will add that shortly. |
comment:16
I am not so sure you guys should give each other positive reviews in this form. I would also either prefer clear instructions on which patches to merge in what order or alternatively a new unified patch. I do not want a bundle. Cheers, Michael |
comment:17
Replying to @sagetrac-mabshoff:
The patches should be merged in order as posted, that is: all_updates_and_cid.patch add_load_file1.patch add_close.patch load_file_bug_fix.patch sage-3621-part5.patch sage-3621-example.patch sage-3621-part6.patch sage-3621-part7.patch sage-3621-part8.patch sage-3621-part9.patch sage-3621-part10.patch Shall I set it back to [with patch, needs review]? |
comment:18
Replying to @cswiercz:
Thanks.
I am talking with William about this, but it seems unclear to me who gave a review to what. For example: Chris posted sage-3621-part10.patch and also gave that patch a positive review. It seems like a minor patch, but we should follow procedure here. Cheers, Michael |
comment:19
Replying to @sagetrac-mabshoff:
To make the situation a bit clearer, Brett asked if I could review his patches. (part6.patch - part9.patch) I found a minor bug and added my own patch at the very end. (part10.patch) My patch was just a small change to a doctest, which I tested thoroughly. I realize just now that it was kinda silly to review this ticket myself since all of the code throughout these patches was done by both of us, equally. Anyway, I'm fine with setting it back to "needs review" and waiting for someone to take a look at it. It's your call. :) -- |
comment:20
Alas, several issues need to be resolved and some features need to be added:
I'm taking care of this right now! :) -- Chris |
Attachment: sage-3621-combined.patch.gz Replacement patch for all patches listed above. Combines all changes. |
comment:21
sage-3621-combined.patch is joined work by William, Chris and Brett. Cheers, Michael |
Attachment: sage-3621-extrapart1.patch.gz Added Yahoo Finance as a backup to Google Finance historical requests. Additional refinements. |
comment:23
Josh, can you review those two patches? Cheers, Michael |
comment:24
Something's up with the first patch -- the file SAGE_ROOT/examples/finance/AAPL-minutely.csv is located at SAGE_ROOT/finance/AAPL-minutely.csv when I apply the patch to a fresh 3.1.2.rc1.
|
comment:25
Oops, my bad -- William already commented on that. |
apply to repository in SAGE_ROOT/examples |
Attachment: 3621–example.patch.gz removes creation of example file from original |
comment:26
Attachment: 3621-combined.patch.gz All (including optional) tests pass! |
Attachment: sage-3621-trivial_referee_followup.patch.gz |
comment:27
REFEREE REPORT: Positive Review. This is superb code. I had to make a few minor changes since for some reason google changed their database. I also updated some of the # optional tags for the new system. Michael, to apply this you must:
That's it. -- William |
comment:28
Merged the patches William mentioned above in 3.2.1.rc0. The credit situation:
Please let me know if I got anything wrong Cheers, Michael |
There was an issue with Stock.historical() only returning stock information from Jan 1, 1990 onward and no earlier. The addition of an explicit enddate to the Google Finance query seems to fix this. Fix is demonstrated in the doctests of Stock.historical()
Uses the datetime.date python module.
CC: @sagetrac-jkantor
Component: finance
Keywords: finance, historical, stock, Stock
Issue created by migration from https://trac.sagemath.org/ticket/3621
The text was updated successfully, but these errors were encountered: