EntilZha / PyFunctional

Python library for creating data pipelines with chain functional programming
http://pyfunctional.pedro.ai
MIT License
2.41k stars 132 forks source link

Fix Bug where _wrap handles Pandas DFs incorrectly #122

Closed geenen124 closed 6 years ago

geenen124 commented 6 years ago

Closes #97

EntilZha commented 6 years ago

Thanks for the PR!

I took a look at the failed tests, and they look unrelated to the PR (version of lint checker upgraded and code hasn't been changed to adapt yet).

Things look good to me with the exception of a cleaner way of skipping the test if pandas isn't installed which I'll mention in a sec. Once that is changed then I'll merge, and fix those random lint errors.

codecov-io commented 6 years ago

Codecov Report

Merging #122 into master will decrease coverage by 1.42%. The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #122      +/-   ##
==========================================
- Coverage    99.9%   98.47%   -1.43%     
==========================================
  Files          12       12              
  Lines        2149     2169      +20     
==========================================
- Hits         2147     2136      -11     
- Misses          2       33      +31
Impacted Files Coverage Δ
functional/pipeline.py 99.25% <71.42%> (-0.75%) :arrow_down:
functional/test/test_functional.py 99.73% <84.61%> (-0.27%) :arrow_down:
functional/io.py 90.9% <0%> (-9.1%) :arrow_down:
functional/transformations.py 96.44% <0%> (-3.56%) :arrow_down:
functional/test/test_streams.py 98.16% <0%> (-1.84%) :arrow_down:
functional/util.py 98.41% <0%> (-1.59%) :arrow_down:

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 9459ee3...60061ae. Read the comment docs.

geenen124 commented 6 years ago

I've used a similar decorator to the one you suggested :+1:

EntilZha commented 6 years ago

Thanks for the fix! Great work, LGTM to merge