Skip to content
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

BUG: fix for GH9010 #9198

Closed
wants to merge 1 commit into from
Closed

BUG: fix for GH9010 #9198

wants to merge 1 commit into from

Conversation

PollyP
Copy link

@PollyP PollyP commented Jan 5, 2015

Superseded by #9358


This is an update of PR #9024.

closes #9010

underlying_price = float(underlying_price)
except ValueError:
# see if there is a comma thousands separator that needs to be filtered out
underlying_price = ''.join(c for c in underlying_price if c != ',')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this should just be underlying_price.replace(',', ''). As noted in the other PR, this method is not deprecated in Python 3: https://docs.python.org/3/library/stdtypes.html#str.replace

@jreback jreback added Bug Data Reader Compat pandas objects compatability with Numpy or Python functions labels Jan 5, 2015
@jreback jreback added this to the 0.16.0 milestone Jan 5, 2015
@jreback
Copy link
Contributor

jreback commented Jan 18, 2015

@PollyP can you rebase, and update according to comments by @shoyer

@PollyP
Copy link
Author

PollyP commented Jan 20, 2015

OK, will do by Saturday at the latest.

On Sun, Jan 18, 2015 at 1:00 PM, jreback notifications@github.com wrote:

@PollyP https://github.com/PollyP can you rebase, and update according
to comments by @shoyer https://github.com/shoyer


Reply to this email directly or view it on GitHub
#9198 (comment).

@PollyP
Copy link
Author

PollyP commented Jan 23, 2015

Looks like dstephens99 went ahead and fixed this.

@PollyP PollyP closed this Jan 23, 2015
@jorisvandenbossche
Copy link
Member

yes, @dstephens99 fixed it in pandas-datareader, but until we decide what exactly to do with the code in pandas, this can maybe still be merged in pandas? (@jreback)

(only reopened to be sure we decide on this, we can just close it again if we are not going to merge this)

@jreback
Copy link
Contributor

jreback commented Jan 25, 2015

see comments here: pydata/pandas-datareader#15

@jreback
Copy link
Contributor

jreback commented Mar 3, 2015

@jorisvandenbossche are we merging this?

@jorisvandenbossche
Copy link
Member

I think we should close this in favor of #9358 (which brings the changes made in pandas-datareader to back to pandas, including this)

@PollyP Thanks for your work on this! If you want to do more fixes in the future, you can submit them directly to pandas-datareader.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Compat pandas objects compatability with Numpy or Python functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Yahoo options DataReader can't parse underlying prices with commas
4 participants