Skip to content

[WIP] Task upload - #607

Merged
mfeurer merged 57 commits into
developfrom
task_upload
Apr 26, 2019
Merged

[WIP] Task upload#607
mfeurer merged 57 commits into
developfrom
task_upload

Conversation

@ArlindKadra

@ArlindKadraArlindKadra commented Dec 8, 2018

Copy link
Copy Markdown
Member

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

  • Implements task upload.

    1. Generates dictionary that represents object.
    2. Generates xml from the dictionary.
  • Adds a function that allows tasks to be created (facilitates task creation for users compared to the task constructor).

  • Adds type annotations for the task object.

How should this PR be tested?

New unit tests for different kinds of tasks.

Addresses #657, #656

@codecov-io

codecov-io commented Jan 29, 2019

Copy link
Copy Markdown

Codecov Report

Merging #607 into develop will decrease coverage by 0.13%.
The diff coverage is 86.91%.

Impacted file tree graph

@@ Coverage Diff @@## develop #607 +/- ##
===========================================
- Coverage 90.67% 90.54% -0.14% 
===========================================
Files 36 36 Lines 3690 3743 +53 ===========================================
+ Hits 3346 3389 +43 - Misses 344 354 +10
Impacted FilesCoverage Δ
openml/datasets/dataset.py88.58% <100%> (+0.03%)⬆️
openml/tasks/__init__.py100% <100%> (ø)⬆️
openml/extensions/sklearn/extension.py90.62% <64.28%> (-0.25%)⬇️
openml/runs/functions.py81.5% <75%> (-0.37%)⬇️
openml/tasks/functions.py85.09% <83.33%> (-0.07%)⬇️
openml/runs/run.py89.65% <83.33%> (-0.51%)⬇️
openml/tasks/task.py94.77% <93.65%> (-1.35%)⬇️

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 f656062...6e70b83. Read the comment docs.

@ArlindKadra
ArlindKadra requested review from mfeurer and removed request for mfeurerFebruary 7, 2019 21:08
Comment threadopenml/tasks/task.py Outdated
Comment threadopenml/tasks/task.py Outdated
Comment threadtests/test_tasks/test_stat_task.py Outdated
Comment threadtests/test_tasks/test_task.py Outdated
Comment threadtests/test_tasks/test_supervised_task.py Outdated
Comment threadtests/test_tasks/test_task.py
Comment threadopenml/tasks/task.py Outdated
Comment threadopenml/tasks/task.py Outdated
task_container = super(OpenMLClusteringTask, self)._to_dict()
task_dict = task_container['oml:task_inputs']

task_dict['oml:input'].append(

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.

Is the estimation procedure common to all task types? If yes, could you please add it to the parent class?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Nice observation. It also made me realise that the code for some of the tasks can be incomplete since when the class is downloaded from the server it has more information concerning the estimation procedure.

Comment threadtests/test_tasks/test_supervised_task.py Outdated
Comment threadtests/test_tasks/test_clustering_task.py Outdated
Comment threadtests/test_tasks/test_task.py
Comment threadtests/test_tasks/test_task.py Outdated
Comment threadtests/test_tasks/test_task.py Outdated

@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.

Hey, this looks really good! Only a few minor comments. I'll approve once the PEP8 stuff is green.

Comment threadopenml/tasks/functions.py Outdated
Comment threadopenml/tasks/functions.py Outdated
Comment threadopenml/tasks/functions.py Outdated
Comment threadopenml/tasks/task.py Outdated
Comment threadtests/test_tasks/test_task.py Outdated
Comment threadtests/test_tasks/test_task.py Outdated
Comment threadopenml/tasks/__init__.py Outdated
Comment threadopenml/tasks/task.py Outdated

@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.

Awesome, I just have a few minor questions/requests, and then we should be good to go with this!

Comment threadopenml/extensions/sklearn/extension.py
Comment threadopenml/runs/functions.py
Comment threadopenml/runs/run.py
Comment threadopenml/runs/run.py
Comment threadopenml/tasks/functions.py Outdated
Comment threadopenml/tasks/task.py Outdated
Comment threadtests/test_tasks/test_clustering_task.py Outdated
@mfeurer
mfeurer merged commit 813daeb into developApr 26, 2019
@mfeurer
mfeurer deleted the task_upload branch April 26, 2019 10:34
@mfeurermfeurer mentioned this pull request Apr 29, 2019
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.

3 participants

@ArlindKadra@codecov-io@mfeurer