Skip to content

Conversation

@Rong-Inspur
Copy link
Contributor

Reference Issue

Fix #871

What does this PR implement/fix? Explain your changes.

Remove 'version' from our all list in openml_init_.py

How should this PR be tested?

Any other comments?

@codecov-io
Copy link

codecov-io commented Feb 25, 2020

Codecov Report

Merging #903 into develop will decrease coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop     #903     +/-   ##
==========================================
- Coverage    88.13%   88.03%   -0.1%     
==========================================
  Files           37       37             
  Lines         4364     4364             
==========================================
- Hits          3846     3842      -4     
- Misses         518      522      +4
Impacted Files Coverage Δ
openml/__init__.py 100% <100%> (ø) ⬆️
openml/_api_calls.py 87.93% <0%> (-2.59%) ⬇️
openml/datasets/functions.py 93.53% <0%> (-0.31%) ⬇️

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 07d429c...0f29caf. Read the comment docs.

@PGijsbers
Copy link
Collaborator

PGijsbers commented Feb 26, 2020

Edit: My bad! Looks like did not see the last commit you made to this PR. Seems like you figured it out. You can make flake8 ignore the unused import by adding an in-line comment # noqa: F401. I think the other unit test failure is unrelated to the changes in this PR.


old comment:

The issue might have been too vague. The desired behavior is to be able to do this:

import openml
print(openml.__version__)  # 0.11.0dev

but not this:

from openml import *
__version__

The latter currently works fine (__version__ evaluates to the version str), but it should instead raise NameError: name '__version__' is not defined.

Using openml.__version__ is the normal way to check a package version. It should remain functional (and is also used in unit tests).

From my understanding this means the only change required would be to remove this line. But you would need to verify that.

Copy link
Collaborator

@PGijsbers PGijsbers 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 to me! Thank you for your contribution 🎉

@PGijsbers PGijsbers merged commit 4b9b873 into openml:develop Feb 27, 2020
@Rong-Inspur
Copy link
Contributor Author

Looks good to me! Thank you for your contribution 🎉

Thanks a lot for the review and suggestions!

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.

__version__ is imported with from openml import *

3 participants