-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[ENH] in notebooks, replace yfinance download with loaders
#678
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: main
Are you sure you want to change the base?
Conversation
update from central main
|
@fkiraly this is good to be merged. Now using one flat file common for all notebooks. I have added the script to generate this file. |
fkiraly
left a comment
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.
Great! I really think we need to replace downloads by onboard data.
- I believe you have to explicitly request
csvto be packaged inpyproject, or users will not be able to load the data. This becomes apparent in the wheels/release test (but we are not running a packaging test regularly). - could you do separate things in separate PR? E.g., linting the notebooks, changing dependencies, etc.
- The pattern that I would use is adding a
load_datasetnamefunction instead of directly loading acsv. That makes the notebook easier to read.
Different topic, I think the pip installs should be removed from the notebooks, this messes up the VM that tests the notebooks.
yfinance download with loaders
|
Plus, could you please, please write descriptive summaries for your PR? Use AI if you need to. |
|
I don't understand. Do you want to include the notebooks in the package released? |
??? why would one have this in a notebook? |
no, I want loader functions for the csv in the package release, and the notebooks then import the loader from the package, rather than loading the from pypfopt.data import load_something
my_dummydata = load_something()All the csv manipulation is hidden underneath, ensuring that the notebook is short and does not distract from what is being shown. |
Get rid of yfinance. Create on csv file serving all notebooks