Skip to content

replace __str__ by __repr__ #743

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.

Sign up for GitHub

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

Jump to bottom
Merged
mfeurer merged 4 commits into from
Jul 25, 2019
Merged

replace __str__ by __repr__ #743

mfeurer merged 4 commits into from
Jul 25, 2019

Conversation

@amueller
Copy link
Contributor

@amueller amueller commented Jul 22, 2019

Closes #739, #75

cc @Neeratyoy, @mfeurer

@amueller
Copy link
Contributor Author

amueller commented Jul 22, 2019

test failure in test_list_datasets_with_high_size_parameter seems unrelated?

@mfeurer
Copy link
Collaborator

mfeurer commented Jul 23, 2019

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
Contributor

Neeratyoy commented Jul 23, 2019

Thanks a lot, @amueller.

mfeurer
mfeurer requested changes Jul 24, 2019
View reviewed changes
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.

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__?

@mfeurer mfeurer mentioned this pull request Jul 24, 2019
Closed
@codecov-io
Copy link

codecov-io commented Jul 24, 2019 •
edited
Loading

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
Contributor Author

amueller commented Jul 24, 2019

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

@mfeurer
Copy link
Collaborator

mfeurer commented Jul 24, 2019

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

@PGijsbers
Copy link
Collaborator

PGijsbers commented Jul 24, 2019

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

@amueller
Copy link
Contributor Author

amueller commented Jul 24, 2019

I might have gotten it.

@amueller
Copy link
Contributor Author

amueller commented Jul 24, 2019

hm still fails on linux on 3.5 :-/

@PGijsbers
Copy link
Collaborator

PGijsbers commented Jul 24, 2019

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
Contributor Author

amueller commented Jul 24, 2019

failures seem unrelated now.

@mfeurer mfeurer merged commit 8eef523 into openml:develop Jul 25, 2019
@mfeurer mfeurer mentioned this pull request Jul 25, 2019
Closed
@amueller
Copy link
Contributor Author

amueller commented Jul 25, 2019

yay thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mfeurer mfeurer mfeurer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@amueller @mfeurer @Neeratyoy @codecov-io @PGijsbers