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

ENH: Add pandas_compat.table_from_frame(df) #2836

Merged
merged 1 commit into from Dec 20, 2017

Conversation

Projects
None yet
4 participants
@kernc
Copy link
Member

commented Dec 19, 2017

Issue

Various third parties work with pandas.DataFrames. Missing was a convenient method of transforming result DataFrame into Orange Table.

Description of changes

Add table_from_frame utility function. No hard pandas dependencies. Please advise on the best way to run the tests.

Includes
  • Code changes
  • Tests
  • Documentation

@kernc kernc force-pushed the kernc:pandas_compat branch from d653e95 to f1105e1 Dec 19, 2017

@astaric

This comment has been minimized.

Copy link
Member

commented Dec 19, 2017

AFAIAC pandas can be installed on travis (and appveyor) to run the tests while not being added to the install_requires listed in setup.py.

I assume this is how pandas tests HDFS support as there are tests in their repository and tables listed in optional-requirements, while it does not get installed when you run pip install pandas.

@codecov-io

This comment has been minimized.

Copy link

commented Dec 19, 2017

Codecov Report

Merging #2836 into master will increase coverage by 0.01%.
The diff coverage is 95.94%.

@@            Coverage Diff             @@
##           master    #2836      +/-   ##
==========================================
+ Coverage   81.96%   81.98%   +0.01%     
==========================================
  Files         321      323       +2     
  Lines       54841    54915      +74     
==========================================
+ Hits        44952    45023      +71     
- Misses       9889     9892       +3

@kernc kernc force-pushed the kernc:pandas_compat branch from 4a59487 to 48828e4 Dec 19, 2017

@jerneju jerneju referenced this pull request Dec 19, 2017

Merged

[FIX] Yahoo! Finance #39

1 of 3 tasks complete
class TestPandasCompat(unittest.TestCase):
def test_table_from_frame(self):
from Orange.data.pandas_compat import Test
Test().test_table_from_frame()

This comment has been minimized.

Copy link
@astaric

astaric Dec 19, 2017

Member

Why don't you move the body of the test here?

@kernc kernc force-pushed the kernc:pandas_compat branch 2 times, most recently from 48a889c to 5b57c80 Dec 19, 2017

def _is_discrete(s):
return (is_categorical_dtype(s) or
is_object_dtype(s) and (force_nominal or
s.nunique() < s.size**.666))

This comment has been minimized.

Copy link
@astaric

astaric Dec 19, 2017

Member

I guess you do not want to use is_discrete_values here as it is too complicated, but you could at least use the same constant :)

This comment has been minimized.

Copy link
@kernc

kernc Dec 20, 2017

Author Member

👍

It's the exact same [universal] constant, just computed with more precision. 😜

metas.append(StringVariable(name))
M.append(s.values.astype(object))

MAX_LENGTH = max(len(X[0]) if X else 0,

This comment has been minimized.

Copy link
@astaric

astaric Dec 19, 2017

Member

Is this ever going to be different than df.shape[0] (or equivalent)?

This comment has been minimized.

Copy link
@kernc

kernc Dec 20, 2017

Author Member

Constructing Table objects, one sometimes gets so invested with details that they tend to lose grasp of the whole picture.

Thanks.

@astaric

This comment has been minimized.

Copy link
Member

commented Dec 20, 2017

The code look ok to me. Squash if you intended to and I'll merge.

Since you ticked the documentation checkbox, should this be mentioned in the docs somewhere? I do not have a specific place in mind so the aswer to this could be "no", or "not yet", but I think it is a useful helper more people should know about :)

@kernc kernc force-pushed the kernc:pandas_compat branch 3 times, most recently from d0a4c8e to 379dd56 Dec 20, 2017

@kernc kernc force-pushed the kernc:pandas_compat branch from 379dd56 to e602be2 Dec 20, 2017

@kernc

This comment has been minimized.

Copy link
Member Author

commented Dec 20, 2017

Was referring to reasonable docstring as provided. Guessing reviewers should be the ones to catch it until DataFrames become a more frequently integrated pattern.

@astaric astaric merged commit 06709e9 into biolab:master Dec 20, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.