Dev docstrings #33

jakubczakon · 2018-05-04T16:32:28Z

No description provided.

…mers

grzes314

Most of my remarks are minor. Some of them follow from dynamic changes in the code. The comments will require rereading anyway once we have a mature version of the library.

grzes314 · 2018-05-16T15:58:28Z

steps/adapters.py

@@ -3,51 +3,157 @@



Since adapter syntax won't allow passing a function any more, purpose of functions in this module should be reconsidered. But for the time being I would leave them as they are.

grzes314 · 2018-05-16T15:59:27Z

steps/base.py

+                     step_3 = Step(name='step_3',
+                                   input_steps=[step_1, step_2],
+                                   input_data=['input_2'],
+                                   adapter = {'X':([('step_1','X_transformed'),


Old adapter syntax.

But for now you can leave it as it is. Step API is probably going to change quite a few times before we get a "mature" version. Let's not waste too much time on polishing comments for each temporary version.

grzes314 · 2018-05-16T16:01:06Z

steps/base.py

+        Args:
+            name (int): Step name. Each step in a pipeline needs to have a unique name.
+                Transformers, and Step outputs will be persisted/cached/saved under this exact name.
+            transformer (obj): Step instance or object that inherits from BaseTransformer.


transformer (BaseTransformer)

@grzes314 it is either BaseTransformer or Step how would you handle that?

Do we really have a usecase when we would like to put a Step here instead of Transformer? Anyway, if we incorporate Michal's recent proposition, Step and Transformer will be unified, so let's forget it.

grzes314 · 2018-05-16T16:15:18Z

steps/base.py

        self.fit(*args, **kwargs)
        return self.transform(*args, **kwargs)

    def load(self, filepath):
+        """Saves the trainable parameters of the transformer


grzes314 · 2018-05-16T16:18:32Z

steps/base.py

+            kwargs: keyword arguments (can be anything)
+
+        Returns:
+            obj: BaseTransformer instance


Returns: BaseTransformer: self object

grzes314 · 2018-05-16T16:21:39Z

steps/utils.py

+    It creates logger of name 'steps'
+
+    Returns:
+        obj: logging.Logger object


Returns: logging.Logger: logger object

grzes314 · 2018-05-16T16:23:47Z

steps/base.py

+        Args:
+            name (str): name of the step to be fetched
+        Returns:
+            obj: Step instance


Returns: Step: extracted step

grzes314 · 2018-05-16T16:24:37Z

steps/utils.py

+
+    Returns:
+        obj: logging.Logger object of name 'steps'


Returns: logging.Logger: logger object

grzes314 · 2018-05-16T16:26:07Z

steps/base.py

+            kwargs: keyword arguments (can be anything)
+
+        Returns:
+            obj: BaseTransformer instance


it returns a dictionary with results, not a BaseTransformer instance

grzes314 · 2018-05-16T16:32:36Z

steps/base.py

+            transformer (obj): Step instance or object that inherits from BaseTransformer.
+                When Step instance is passed transformer from that Step will be copied and used to perform transformations.
+                It is useful when both train and valid data are passed in one pipeline (common situation in deep learnining).
+            input_steps (list): list of Step instances default []. Current step will combine outputs from input_steps and input_data


I changed the default to None on one of the branches

@grzes314 can I change it to None leaving everything else as is ?

On second thought, it might be cleaner to change such things after merge. Current comment is consistent with code on this branch.

kamil-kaczmarek

ok

* initial * cleaned up * updates keras models and callbacks * added explanation notebook * renamed notebook * ran notebook * transformer loading now to be clicked * dropped nlp * fixed typos * updated whats missing * updated with dsb and talking data * added simple pipeline notebook * imports optimized * feature_names=None and categorical_features=None * refactor the way the model parameters are passed * moved adapters to seperate file, renamed identity/take_first adapters * Update misc.py * fit_transform -> transform * Update base.py * fixed #10 * fixed typo * * added: get_upsample_pad, get_downsample_pad * added: multiclass_segmentation_loss * *warning added * Add some user interface messages * Notebook#3: Adapter for ensembling * Add example: 1-getting-started.ipynb * Remove custom adapter; other minor changes after reviewer feedback * Add example 2-multi-step.ipynb and some minor changes to example 1-getting-started.ipynb * Cleanup of examples 1 and 2 * Fix a mistake in notebook text * Add steps/__pycache__ to .gitignore * Further changes based on Github feedback * add self.conv_stride parameter in unet definition * Change steps/__pycache__ to *.pyc in .gitignore * Changes in Notebook#3 following code review * train forests on train data and ensembler on ensembling data * decoupled sklearn preprocessing from text preprocessing with heavy de… (#32) * decoupled sklearn preprocessing from text preprocessing with heavy dependencies * fixed import error * dropped anonymization from text.py, reformatted imports, small refactors * small refactor for readability * Change time format (#40) * Dev docstrings (#33) * Step halfway done * Step init args done * Steps docstrings done * added docstrings to Step properties * Step docstring done * dropped mock transformer addeed docstrings to Base and Dummy Transformers * base documentation finished, minor refactors * docstrings added to adapters * docstrings added * started refactoring lgbm * Update base.py * updated docstrings * base transformer docstring fix * fixed logging docstrings * Notebook #5: Example with Keras (#41) * Notebook #5: Example with Keras * Corrected notebook for Keras and necessary refactors in ClassifierGenerator * Fix issue #28: Unintuitive adapter syntax (#42) * Write tests for new adapter syntax * Refactor adapter * Improve handling of caches and logs in tests * Fix minor issues mentioned in PR comments * Rewrite tests in pytest framework * Move adapting to seperate class, alter behaviour * Correction: mutable object as default argument in Step initializer * Issue #16: make_transformer (#50) * Corrections and tests for Sklearn wrappers * make_transformer taking arbitrary functions * fix issues #48 and #49 (#52) * fix issues #48 and #49 * use of setdefault method * fix according to Kamil's request * Fix post merge issues * Prepare package (#58) * Remove tutorial notebooks * Remove modules with heavy dependencies * Rename steps -> steppy * Add setup.py * Tell git to ignore egg files * Read the docs might output of sphinx-quickstart * Requirements file * Try adding sphinx-apidoc output * Add some requirements * Add IPython to requirements * removed intro.ipynb and updated requirements.txt * corrected setup.py * refactored NoOperation to IdentityOperation, optimized inputs * moved misc.py to steppy-toolkit * simplified ProbabilityCalibation, BaseTransformer small refactor * prepare setup.py and setup.cfg for PyPI registration * added install requires to setup.py * name refactor * Create README.md

cubestone and others added 12 commits April 21, 2018 11:21

Step halfway done

4eedcc9

Step init args done

56d7c34

Steps docstrings done

e5ca128

added docstrings to Step properties

6ddb090

Step docstring done

450bcc2

dropped mock transformer addeed docstrings to Base and Dummy Transfor…

3c84630

…mers

base documentation finished, minor refactors

65a70bb

docstrings added to adapters

b6a1f51

docstrings added

6d8c3a6

started refactoring lgbm

8830fd6

added docstrings

bd2f529

Merge branch 'dev' into dev-docstrings

e6c5bc1

kamil-kaczmarek self-assigned this May 7, 2018

kamil-kaczmarek added this to the Steps as Python package milestone May 7, 2018

kamil-kaczmarek self-requested a review May 7, 2018 11:48

kamil-kaczmarek added the tutorials/docs label May 7, 2018

jakubczakon assigned grzes314 and mromaniukcdl May 9, 2018

jakubczakon requested review from grzes314 and mromaniukcdl May 9, 2018 19:50

grzes314 reviewed May 16, 2018

View reviewed changes

jakubczakon and others added 4 commits May 16, 2018 18:39

Update base.py

c0509d2

updated docstrings

d2432a9

base transformer docstring fix

562d8ff

fixed logging docstrings

865aa2b

grzes314 approved these changes May 17, 2018

View reviewed changes

kamil-kaczmarek approved these changes May 17, 2018

View reviewed changes

Merge branch 'dev' into dev-docstrings

b386daa

kamil-kaczmarek merged commit 1e577fa into dev May 17, 2018

kamil-kaczmarek deleted the dev-docstrings branch May 17, 2018 12:13

kamil-kaczmarek mentioned this pull request May 23, 2018

dev-0.1.1 #60

Merged

kamil-kaczmarek unassigned grzes314 and mromaniukcdl Jun 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev docstrings #33

Dev docstrings #33

jakubczakon commented May 4, 2018

grzes314 left a comment

grzes314 May 16, 2018

grzes314 May 16, 2018

jakubczakon May 16, 2018

grzes314 May 16, 2018

jakubczakon May 16, 2018

grzes314 May 17, 2018

grzes314 May 16, 2018

grzes314 May 16, 2018

grzes314 May 16, 2018

grzes314 May 16, 2018

grzes314 May 16, 2018

grzes314 May 16, 2018

grzes314 May 16, 2018

jakubczakon May 16, 2018

grzes314 May 17, 2018

kamil-kaczmarek left a comment

Dev docstrings #33

Dev docstrings #33

Conversation

jakubczakon commented May 4, 2018

grzes314 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamil-kaczmarek left a comment

Choose a reason for hiding this comment