Skip to content

replace __str__ by __repr__#743

Merged
mfeurer merged 4 commits into
openml:developfrom
amueller:repr_no_str
Jul 25, 2019
Merged

replace __str__ by __repr__#743
mfeurer merged 4 commits into
openml:developfrom
amueller:repr_no_str

Conversation

@amueller

Copy link
Copy Markdown
Contributor

Closes #739, #75

cc @Neeratyoy, @mfeurer

@amueller

Copy link
Copy Markdown
Contributor Author

test failure in test_list_datasets_with_high_size_parameter seems unrelated?

@mfeurer

mfeurer commented Jul 23, 2019

Copy link
Copy Markdown
Collaborator

test failure in test_list_datasets_with_high_size_parameter seems unrelated?

yep, it is. This one might fail sporadically if we concurrently delete stuff from the test server.

@Neeratyoy

Copy link
Copy Markdown
Contributor

Thanks a lot, @amueller.

@mfeurer mfeurer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for your suggestion @amueller and the PR

However, it turns out that this now makes a unit test fail on Windows. Would you mind updating the test REGEX to accomodate for the change from __str__ to __repr__?

@codecov-io

codecov-io commented Jul 24, 2019

Copy link
Copy Markdown

Codecov Report

Merging #743 into develop will increase coverage by 0.08%.
The diff coverage is 87.5%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #743      +/-   ##
===========================================
+ Coverage    87.89%   87.98%   +0.08%     
===========================================
  Files           36       36              
  Lines         4033     4077      +44     
===========================================
+ Hits          3545     3587      +42     
- Misses         488      490       +2
Impacted Files Coverage Δ
openml/datasets/data_feature.py 70% <100%> (ø) ⬆️
openml/flows/flow.py 93.8% <100%> (ø) ⬆️
openml/evaluations/evaluation.py 61.53% <100%> (ø) ⬆️
openml/datasets/dataset.py 84.07% <100%> (ø) ⬆️
openml/exceptions.py 93.75% <100%> (-3.13%) ⬇️
openml/tasks/task.py 81.01% <100%> (ø) ⬆️
openml/setups/setup.py 42.3% <100%> (ø) ⬆️
openml/runs/run.py 85.71% <100%> (ø) ⬆️
openml/runs/trace.py 90.75% <100%> (ø) ⬆️
openml/study/study.py 64.36% <60%> (ø) ⬆️
... and 4 more

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 56fcc00...58f03f4. Read the comment docs.

@amueller

Copy link
Copy Markdown
Contributor Author

Do you have an idea of why this fails on windows? That should be working, right?

@mfeurer

mfeurer commented Jul 24, 2019

Copy link
Copy Markdown
Collaborator

Nope, maybe @PGijsbers knows. He (together with @WilliamRaynaut) set up the Windows tests and had some regex issues back then, too.

@PGijsbers

Copy link
Copy Markdown
Collaborator

I'm not sure. I tried the tests locally and they work on my Windows machine.

@amueller

Copy link
Copy Markdown
Contributor Author

I might have gotten it.

@amueller

Copy link
Copy Markdown
Contributor Author

hm still fails on linux on 3.5 :-/

@PGijsbers

Copy link
Copy Markdown
Collaborator

I might have gotten it.

I saw you were working on fixing it, so I checked against older versions too. All commits in this PR left me with passing unit tests (at least those that FAILED on Appveyor when I was pinged).

@amueller

Copy link
Copy Markdown
Contributor Author

failures seem unrelated now.

@mfeurer mfeurer merged commit 8eef523 into openml:develop Jul 25, 2019
@mfeurer mfeurer mentioned this pull request Jul 25, 2019
@amueller

Copy link
Copy Markdown
Contributor Author

yay thank you!

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.

5 participants