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

@amueller

Copy link
Copy Markdown
ContributorAuthor

test failure in test_list_datasets_with_high_size_parameter seems unrelated?

@mfeurer

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.

@mfeurermfeurer 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 FilesCoverage Δ
openml/datasets/data_feature.py70% <100%> (ø)⬆️
openml/flows/flow.py93.8% <100%> (ø)⬆️
openml/evaluations/evaluation.py61.53% <100%> (ø)⬆️
openml/datasets/dataset.py84.07% <100%> (ø)⬆️
openml/exceptions.py93.75% <100%> (-3.13%)⬇️
openml/tasks/task.py81.01% <100%> (ø)⬆️
openml/setups/setup.py42.3% <100%> (ø)⬆️
openml/runs/run.py85.71% <100%> (ø)⬆️
openml/runs/trace.py90.75% <100%> (ø)⬆️
openml/study/study.py64.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
ContributorAuthor

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

@mfeurer

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
ContributorAuthor

I might have gotten it.

@amueller

Copy link
Copy Markdown
ContributorAuthor

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
ContributorAuthor

failures seem unrelated now.

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

Copy link
Copy Markdown
ContributorAuthor

yay thank you!

Sign up for freeto 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

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