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

read_sql_table - datetime index fix and index type checking #4474

Merged
merged 5 commits into from
Feb 26, 2019

Conversation

jcorb
Copy link
Contributor

@jcorb jcorb commented Feb 11, 2019

Fixes #4289

Also added a type check on the index column if divisions is None. It will raise a TypeError if the index column's dtype.kind is not in ['i', 'u', 'f'] or equal to M

  • Tests added / passed
  • Passes flake8 dask

@martindurant
Copy link
Member

Could you please add a test, in which a text column is chosen, and so it raises the exception?

@jcorb
Copy link
Contributor Author

jcorb commented Feb 13, 2019

Yes, sure thing. I presume it would sit in here dask/dask/dataframe/io/tests/test_sql.py?

@martindurant
Copy link
Member

Yes exactly

@jcorb
Copy link
Contributor Author

jcorb commented Feb 26, 2019

apologies for the delay - I have added a test which sets the index column to be a character column and checks for the TypeError exception.

(first time I have written that sort of test, so I just mimicked what was in there already - hopefully it is along the right lines)

@martindurant
Copy link
Member

Looks good, except lint:

dask/dataframe/io/tests/test_sql.py:266:84: W292 no newline at end of file

@jcorb
Copy link
Contributor Author

jcorb commented Feb 26, 2019

Ah, missed that step - should be good now.

@martindurant martindurant merged commit 6ecbb0c into dask:master Feb 26, 2019
@martindurant
Copy link
Member

Thank you.

@jrbourbeau jrbourbeau mentioned this pull request Apr 17, 2019
2 tasks
jorge-pessoa pushed a commit to jorge-pessoa/dask that referenced this pull request May 14, 2019
* switched /npartitions and total_seconds()

* added check for type before creating divisions

* removed comments

* added test for character index type error

* added blank line at end of file
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