Skip to content

add tag_entity as backend#667

Merged
mfeurer merged 6 commits intoopenml:developfrom
sahithyaravi:fix619_tagfunctions
Apr 11, 2019
Merged

add tag_entity as backend#667
mfeurer merged 6 commits intoopenml:developfrom
sahithyaravi:fix619_tagfunctions

Conversation

@sahithyaravi
Copy link
Copy Markdown
Member

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

modified push_tag and remove_tag functions to call tag_entity

How should this PR be tested?

Any other comments?

@mfeurer
Copy link
Copy Markdown
Collaborator

mfeurer commented Apr 9, 2019

Thanks a lot for your contribution @sahithyaravi1493, this looks like a useful improvement. However, the static style checker in the continuous integration system complains about an unused import. Could you please check that?

Should fix #619.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 9, 2019

Codecov Report

Merging #667 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #667      +/-   ##
===========================================
+ Coverage    90.84%   90.85%   +<.01%     
===========================================
  Files           36       36              
  Lines         3539     3531       -8     
===========================================
- Hits          3215     3208       -7     
+ Misses         324      323       -1
Impacted Files Coverage Δ
openml/tasks/task.py 95.78% <100%> (-0.09%) ⬇️
openml/datasets/dataset.py 88.51% <100%> (-0.04%) ⬇️
openml/flows/flow.py 94.05% <100%> (-0.13%) ⬇️
openml/runs/run.py 90.15% <100%> (-0.04%) ⬇️
openml/utils.py 92.85% <0%> (+0.79%) ⬆️

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 3dc6dee...e13f540. Read the comment docs.

@mfeurer
Copy link
Copy Markdown
Collaborator

mfeurer commented Apr 9, 2019

Sorry, but there's still an unused import. You can perform this check locally be running bash ci_scripts/flake_diff.sh.

@sahithyaravi
Copy link
Copy Markdown
Member Author

Sorry, but there's still an unused import. You can perform this check locally be running bash ci_scripts/flake_diff.sh.

No problem, I will run this locally and commit.

@sahithyaravi
Copy link
Copy Markdown
Member Author

sahithyaravi commented Apr 10, 2019

Sorry, but there's still an unused import. You can perform this check locally be running bash ci_scripts/flake_diff.sh.

No problem, I will run this locally and commit.

Ignore my recent commits, there seem to be some more errors.

I could not get flake8 locally to work due to some issues. So I fixed the errors thrown by pyflakes and pep8. However, there seem to be additional errors thrown here. Will check on this.

@mfeurer
Copy link
Copy Markdown
Collaborator

mfeurer commented Apr 10, 2019

The new errors appear to be errors raised by mypy.

Would it help you if the output of the two checkers would be separated?

@sahithyaravi
Copy link
Copy Markdown
Member Author

The new errors appear to be errors raised by mypy.

Would it help you if the output of the two checkers would be separated?

Hi Matthias, Yes it would help.

I am unable to run flake8 locally, due to an error caused by a space in my user folder. It seemed to be a bit tricky to rename the user folder. So, I tried to run pyflakes and fix the warnings, assuming it would fix the import errors, but those fixes now resulted in mypy errors.

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@sahithyaravi sahithyaravi requested a review from mfeurer April 11, 2019 10:29
Copy link
Copy Markdown
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.

This looks good, thanks a lot!

@mfeurer mfeurer merged commit 8cc1436 into openml:develop Apr 11, 2019
@mfeurer mfeurer mentioned this pull request Apr 11, 2019
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.

3 participants