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

Enable empty string to be set as missingValues in inferCellType #1192 #1208

Open
wants to merge 3 commits into
base: master
from

Conversation

@zyzhu
Copy link
Contributor

zyzhu commented Sep 10, 2018

Attempt to fix #1192
Basically I changed the order of the if statement. It shall prioritize checking values set in missingValues even if they contains empty string.
If empty string is not set in missingValues, it will still fall back to check String.IsNullOrWhiteSpace

@ovatsus
Copy link
Member

ovatsus commented Oct 7, 2018

I need to look a bit more into this, because we were already setting it to InteredType.Null, and that should maybe be convertible to float. Can you push a unit test that reproes this fix then I'll see if I can fix it in a different way?

@zyzhu
Copy link
Contributor Author

zyzhu commented Oct 9, 2018

@ovatsus I've added a test case.

@ovatsus
Copy link
Member

ovatsus commented Oct 14, 2018

Thanks. I'll have a look to see if I can fix this by changing the merging of inferred types

@dsyme
Copy link
Contributor

dsyme commented Mar 21, 2019

Close/open to re-run CI

@dsyme dsyme closed this Mar 21, 2019
@dsyme dsyme reopened this Mar 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.