Skip to content
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.

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

wip: addressing comments #159

Merged
merged 14 commits into from
Oct 31, 2019
Merged

wip: addressing comments #159

merged 14 commits into from
Oct 31, 2019

Conversation

angela97lin
Copy link
Contributor

@angela97lin angela97lin commented Oct 28, 2019

Addressing comments for Pipeline v2.0 PR (#108)

@codecov
Copy link

codecov bot commented Oct 29, 2019

Codecov Report

❗ No coverage uploaded for pull request base (pipeline_v2@8eb43df). Click here to learn what that means.
The diff coverage is 93.58%.

Impacted file tree graph

@@              Coverage Diff               @@
##             pipeline_v2     #159   +/-   ##
==============================================
  Coverage               ?   96.58%           
==============================================
  Files                  ?       88           
  Lines                  ?     2140           
  Branches               ?        0           
==============================================
  Hits                   ?     2067           
  Misses                 ?       73           
  Partials               ?        0
Impacted Files Coverage Δ
evalml/pipelines/regression/random_forest.py 100% <ø> (ø)
...ml/pipelines/classification/logistic_regression.py 100% <ø> (ø)
evalml/pipelines/components/utils.py 97.87% <ø> (ø)
evalml/models/auto_base.py 92.16% <ø> (ø)
...valml/pipelines/components/estimators/estimator.py 76.47% <ø> (ø)
evalml/problem_types/utils.py 100% <ø> (ø)
evalml/pipelines/regression/linear_regression.py 100% <100%> (ø)
evalml/pipelines/classification/random_forest.py 100% <100%> (ø)
...l/pipelines/components/transformers/transformer.py 100% <100%> (ø)
evalml/tests/component_tests/test_components.py 100% <100%> (ø)
... and 5 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 8eb43df...d45ef64. Read the comment docs.

@angela97lin
Copy link
Contributor Author

Note: currently investigating scikit-learn/scikit-learn#5523

Copy link
Collaborator

@jeremyliweishih jeremyliweishih left a comment

Choose a reason for hiding this comment

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

Everything looks good but maybe we can add a test checking if input feature names retains the column headers for the set pipelines for now.

@angela97lin angela97lin merged commit 81fd22c into pipeline_v2 Oct 31, 2019
@angela97lin angela97lin deleted the pv2_review branch October 31, 2019 21:55
angela97lin added a commit that referenced this pull request Nov 4, 2019
* adding skeleton for component_base

* oops, added nested components folders, fixing

* linting

* adding skeleton estimator/transfomer classes

* adding basic estimator components for merge merge

* WIP: Components (#107)

* Added imputer

* Clean up imputer

* added onehot and standard scaler

* Need fix selectfrommodel and validating

* Add selectfrommodel

* Cleanup and added basic init test

* lint

* cleaning up, more merging, combining tests

* adding componenttype enum, a little more merging

* linting

* Moved to estimator

* lint fix :P

* pipeline v2 base

* pushing new base for pipeline and simple test

* Faulty scaler for LR

* Working LR

* Broken RF

* Fixed SelectFromModel

* Added RF pipeline

Fixed SelectFromModel

* changing xgboost to use our components

* Added RF Regression

* beginning to fix broken pieces

* continuing to fix tests

* fixing tests in autoclassifier

* linting

* fixing silly typo bug

* linting

* cleaning up pipeline and pipelinebase classes

* adding describe to components and pipeline

* linting and fixing minor bug

* adding check for estimator in pipeline

* Pipeline indexing (#118)

* Added indexing and basic tests

* Switched to pipelinebase for slicing

* Clean up and add docstrings

* lint

* lint again

* Clean up and add error for setting

* adding default value to next() to prevent StopIteration error

* oops, actually fixing...

* fixing docstrings and cleaning imports

* adding simple tests for describe

* linting

* Autogenerated pipeline names (#122)

* Basic name without check

* Add assert

* lint

* Changed name format and name constants

* Cleanup

* moving files to subfolders and removing hyperparameters as class var

* updating file hierarchy

* linting

* removing duplicate

* Add feature_importance tests

* adding extra tests

* adding test, cleanup

* Added linear regression and test for pipeling fitting (#131)

* Added linear regression pipeline and added test for fitting

* Remove unnecessary fit for xgboost component  (#148)

* Remove fit for xgboost

* Remove kwargs

* addressing pr comments: rename, abstract feature_importances, del __init__, cleanup, etc. (#149)

* addressing pr comments: rename, del __init__, cleanup

* cleaning up describe

* feature importances + cleanup of components, added subclasses encoder + feature_selector

* linting, fixing errors

* adding less specific version

* addressing comments

* import errors

* String and component_type component (#153)

* added handling str and component type for component list

* Adding model_type / problem_type to PipelineBase to allow inference (#157)

* adding problem_type and model_type to pipeline base

* problem_type --> problem_types

* cleaning up self.component_list and init pipeline, typo

* forgot to remove print

* removing comments

* changelog

* removing generic SelectFromModel

* Jeremy changes (#164)

* cleanup components utils

* Switch to category_encoder and cleanup RF Select

* add feature_importance to estimator

* More switching to CE

* Move parameters to class attributes and make test

* lint

* Separate changelog test

* Text cleanup

* wip: addressing comments (#159)

* wip: addressing comments

* feature names?

* adding fix for no estimator in generate_name

* fixing test

* addressing more comments on feature_importance

* feature_importances fixed and cleaned :)

* oops, missed merge conflict

* change of name

* minor cleanup

* adding basic test for two feature selectors and adding default components in DEFAULT_COMPONENTS

* adding tests for retaining feature names in input_feature_names

* addressing comments

* cleanup

* fixing
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.

None yet

2 participants