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

[upload csv to hive] fix call to unicodereader.next() #4804

Merged

Conversation

timifasubaa
Copy link
Contributor

@timifasubaa timifasubaa commented Apr 11, 2018

Calling unicodeReader.next() was problematic because it works on python2 but not python3. The correct way to get the first item in the list is next(items) or for i in items. next(items) works in both python2 and 3.

@michellethomas @john-bodley

@codecov-io
Copy link

Codecov Report

Merging #4804 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4804   +/-   ##
=======================================
  Coverage   72.34%   72.34%           
=======================================
  Files         208      208           
  Lines       15511    15511           
  Branches     1202     1202           
=======================================
  Hits        11221    11221           
  Misses       4287     4287           
  Partials        3        3
Impacted Files Coverage Δ
superset/db_engine_specs.py 51.89% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f1d754...91b4498. Read the comment docs.

@john-bodley
Copy link
Member

@timifasubaa it seems in Python 2 uses reader.next() whereas Python 3 uses next(reader). Is this what you mean by being unreliable?

For Python 2 there is a next(reader) method however the logic differs.

@timifasubaa
Copy link
Contributor Author

timifasubaa commented Apr 11, 2018

@john-bodley yes. Thanks for clarifying. I updated the PR description.
I tested on python2 and 3 and it works fine.

@mistercrunch mistercrunch merged commit 20f46ee into apache:master Apr 11, 2018
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
timifasubaa added a commit to timifasubaa/incubator-superset that referenced this pull request May 31, 2018
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.25.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.25.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants