Skip to content
This repository has been archived by the owner on Feb 4, 2021. It is now read-only.

Add python read_table api #10

Merged
merged 3 commits into from
Oct 30, 2018
Merged

Add python read_table api #10

merged 3 commits into from
Oct 30, 2018

Conversation

relud
Copy link
Collaborator

@relud relud commented Oct 25, 2018

No description provided.

@codecov-io
Copy link

Codecov Report

Merging #10 into master will decrease coverage by 3.49%.
The diff coverage is 73.91%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #10     +/-   ##
=========================================
- Coverage   99.12%   95.63%   -3.5%     
=========================================
  Files           6        6             
  Lines         229      275     +46     
  Branches       28       39     +11     
=========================================
+ Hits          227      263     +36     
- Misses          1        8      +7     
- Partials        1        4      +3
Impacted Files Coverage Δ
python/src/mozdata/mozdata.py 100% <100%> (ø) ⬆️
python/src/mozdata/utils.py 80% <69.23%> (-10.48%) ⬇️

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 d5e6b1f...02e15e1. Read the comment docs.

Copy link

@acmiyaguchi acmiyaguchi left a comment

Choose a reason for hiding this comment

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

Sorry about the slow review, this looks great. The tests are solid (but are missing the uri-based calls). I left some comments on things that gave me a little bit of trouble.

python/src/mozdata/utils.py Show resolved Hide resolved
python/tests/test_read_table.py Show resolved Hide resolved
self.sql_table_name += "_" + version

if uri is not None:
self.version = uri_version(uri)

Choose a reason for hiding this comment

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

This will be covered with the write_table method, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes

acmiyaguchi and others added 2 commits October 30, 2018 10:04
Co-Authored-By: relud <dthorn@mozilla.com>
Co-Authored-By: relud <dthorn@mozilla.com>
@relud relud merged commit aa47ccc into master Oct 30, 2018
@relud relud deleted the python_read_table branch October 30, 2018 17:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants