Skip to content

Conversation

@PGijsbers
Copy link
Collaborator

Notes on the fix in #304.

@codecov-io
Copy link

codecov-io commented Mar 3, 2019

Codecov Report

Merging #635 into develop will decrease coverage by 0.26%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #635      +/-   ##
===========================================
- Coverage    90.12%   89.86%   -0.27%     
===========================================
  Files           32       32              
  Lines         3232     3236       +4     
===========================================
- Hits          2913     2908       -5     
- Misses         319      328       +9
Impacted Files Coverage Δ
openml/flows/sklearn_converter.py 90.48% <100%> (+0.08%) ⬆️
openml/_api_calls.py 84.41% <0%> (-3.9%) ⬇️
openml/tasks/functions.py 86.45% <0%> (-1.94%) ⬇️
openml/runs/functions.py 86.4% <0%> (-0.66%) ⬇️

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 98a73b3...b7686bc. Read the comment docs.

@PGijsbers PGijsbers requested a review from mfeurer March 4, 2019 10:37
Copy link
Collaborator

@mfeurer mfeurer left a comment

Choose a reason for hiding this comment

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

Looks good, could you please add a unit test ensuring the behavior and also add a check that numpy arrays cannot be serialized?

@PGijsbers
Copy link
Collaborator Author

@mfeurer I assumed you would get a ping automatically, but I am not sure if this is the case. I added the tests.

@mfeurer
Copy link
Collaborator

mfeurer commented Mar 5, 2019

I get a ping for a commit, but I usually do not check the commits because I don't know if that commit makes it ready for review again. Will check now.

@mfeurer mfeurer merged commit 96ddc13 into develop Mar 5, 2019
@PGijsbers PGijsbers mentioned this pull request Mar 5, 2019
@PGijsbers PGijsbers deleted the fix304 branch March 17, 2019 12:42
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.

4 participants