Skip to content

Conversation

@woodb
Copy link

@woodb woodb commented Jun 26, 2013

Documentation for DataReader had noted that Fama/French was "ff" for data_source, rather than "famafrench".

Modified DataReader slightly to accept "ff" anyway.

Documentation for DataReader had noted that Fama/French was "ff" for DataSource, rather than "famafrench". 

Modified DataReader slightly to accept "ff" anyway.
@cpcloud
Copy link
Member

cpcloud commented Jun 26, 2013

this needs a test. caveat: you would need to write the DataReader test suite since one does not currently exist. i'm cleaning up data.py and i've added a bare bones test suite for DataReader, so i can merge this after the cleanup..or can merge this first and add to the new test suite...@jreback?

@woodb
Copy link
Author

woodb commented Jun 26, 2013

If you want to merge your test suite for DataReader, I can pull that and update it to include a test for the "ff"/"famafrench" option.

@cpcloud
Copy link
Member

cpcloud commented Jun 26, 2013

#4047 ... i'll wait till it passes then merge, then u should rebase and then we can merge your pr

@cpcloud
Copy link
Member

cpcloud commented Jun 26, 2013

also please add a blurb to release notes (they are in doc/source/release.rst) and copy it to doc/source/v0.12.0.txt maybe under the "Enhancement to existing features" heading(s)

@woodb
Copy link
Author

woodb commented Jun 28, 2013

I won't have time until next week to get more into this commit, but this should get us started.

I think it's good enough for this PR, and more advanced testing (i.e. value checking) should probably be another tracked issue.

@woodb woodb mentioned this pull request Jun 29, 2013
@woodb
Copy link
Author

woodb commented Jun 29, 2013

I'm going to close this one for now, I'll reopen a PR with the changes after the dust settles on data.py. Cheers!

@woodb woodb closed this Jun 29, 2013
@woodb woodb deleted the patch-1 branch June 29, 2013 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants