Repository: pdpipe/pdpipe

Issue:
Application context objects should not be kept by default + add way to supply fit context
pdpipe uses `PdpApplicationContext` objects in two ways:
1. As the `fit_context` that should be kept as-is after a fit, and used by stages to pass to one another parameters that should also be used on transform time.
2. As the `application_context` that should be discarded after a specific application is done, and is used by stages to feed consecutive stages with context. It can be added to by supplying `apply(context={})`, `fit_transform(context={})` or `transform(context={})` with a `dict` that will be used to update the application context.

Two changes are required:
1. At the moment there is a single `context` parameter to application functions that is used to update both the fit and the application context. I think they should be two, one for each type of context.
2. At the moment the `application_context` is not discarded when the application is done. It's as simple as `self.application_context = None` expression added at the `PdPipeline` level in the couple of right cases.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/pdpipe/pdpipe/pull/90?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#90](https://codecov.io/gh/pdpipe/pdpipe/pull/90?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (fb406fb) into [master](https://codecov.io/gh/pdpipe/pdpipe/commit/9d04fdd75736e37d576eebb575b28f4df667e9d8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (9d04fdd) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pdpipe/pdpipe/pull/90/graphs/tree.svg?width=650&height=150&src=pr&token=SB4uE4HWbv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)](https://codecov.io/gh/pdpipe/pdpipe/pull/90?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

```diff
@@            Coverage Diff            @@
##            master       #90   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        23           
  Lines         3149      3149           
  Branches       559       559           
=========================================
  Hits          3149      3149           
```


| [Impacted Files](https://codecov.io/gh/pdpipe/pdpipe/pull/90?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [pdpipe/core.py](https://codecov.io/gh/pdpipe/pdpipe/pull/90/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGRwaXBlL2NvcmUucHk=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pdpipe/pdpipe/pull/90?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pdpipe/pdpipe/pull/90?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Last update [9d04fdd...fb406fb](https://codecov.io/gh/pdpipe/pdpipe/pull/90?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).


Patch:
diff --git a/.gitignore b/.gitignore
index ca9049f..8b436c5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -104,3 +104,6 @@ tags
 # don't like pipfiles
 Pipfile
 Pipfile.lock
+
+# Temporary testing file
+notebooks/debug*.py
diff --git a/pdpipe/core.py b/pdpipe/core.py
index 0eb75f8..36ee8c8 100644
--- a/pdpipe/core.py
+++ b/pdpipe/core.py
@@ -810,7 +810,8 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         raise NotImplementedError
 
     def _post_transform_lock(self):
-        self.application_context.lock()
+        # Application context is discarded after pipeline application
+        self.application_context = None
         self.fit_context.lock()
 
     def apply(
@@ -819,7 +820,8 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         exraise: Optional[bool] = None,
         verbose: Optional[bool] = False,
         time: Optional[bool] = False,
-        context: Optional[dict] = {},
+        fit_context: Optional[dict] = {},
+        application_context: Optional[dict] = {},
     ):
         """Applies this pipeline stage to the given dataframe.
 
@@ -844,10 +846,14 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         time : bool, default False
             If True, per-stage application time is measured and reported when
             pipeline application is done.
-        context : dict, optional
+        fit_context : dict, option
+            Context for the entire pipeline, is retained after the pipeline
+            application is completed.
+        application_context : dict, optional
             Context to add to the application context of this call. Can map
             str keys to arbitrary object values to be used by pipeline stages
-            during this pipeline application.
+            during this pipeline application. Discarded after pipeline
+            application.
 
         Returns
         -------
@@ -860,7 +866,7 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
                 exraise=exraise,
                 verbose=verbose,
                 time=time,
-                context=context,
+                application_context=application_context,
             )
             return res
         res = self.fit_transform(
@@ -868,7 +874,8 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
             exraise=exraise,
             verbose=verbose,
             time=time,
-            context=context,
+            fit_context=fit_context,
+            application_context=application_context,
         )
         return res
 
@@ -878,12 +885,13 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         y: Optional[Iterable] = None,
         exraise: Optional[bool] = None,
         verbose: Optional[bool] = False,
-        context: Optional[dict] = {},
+        fit_context: Optional[dict] = {},
+        application_context: Optional[dict] = {},
     ):
         self.fit_context = PdpApplicationContext()
-        self.fit_context.update(context)
+        self.fit_context.update(fit_context)
         self.application_context = PdpApplicationContext()
-        self.application_context.update(context)
+        self.application_context.update(application_context)
         inter_x = X
         times = []
         prev = time.time()
@@ -918,7 +926,8 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         exraise: Optional[bool] = None,
         verbose: Optional[bool] = False,
         time: Optional[bool] = False,
-        context: Optional[dict] = {},
+        fit_context: Optional[dict] = {},
+        application_context: Optional[dict] = {},
     ):
         """Fits this pipeline and transforms the input dataframe.
 
@@ -942,10 +951,14 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         time : bool, default False
             If True, per-stage application time is measured and reported when
             pipeline application is done.
-        context : dict, optional
+        fit_context : dict, option
+            Context for the entire pipeline, is retained after the pipeline
+            application is completed.
+        application_context : dict, optional
             Context to add to the application context of this call. Can map
             str keys to arbitrary object values to be used by pipeline stages
-            during this pipeline application.
+            during this pipeline application. Discarded after pipeline
+            application.
 
         Returns
         -------
@@ -954,12 +967,15 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         """
         if time:
             return self.__timed_fit_transform(
-                X=X, y=y, exraise=exraise, verbose=verbose, context=context)
+                X=X, y=y, exraise=exraise,
+                verbose=verbose,
+                fit_context=fit_context,
+                application_context=application_context)
         inter_x = X
         self.application_context = PdpApplicationContext()
-        self.application_context.update(context)
+        self.application_context.update(application_context)
         self.fit_context = PdpApplicationContext()
-        self.fit_context.update(context)
+        self.fit_context.update(fit_context)
         for i, stage in enumerate(self._stages):
             try:
                 stage.fit_context = self.fit_context
@@ -985,7 +1001,8 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         exraise: Optional[bool] = None,
         verbose: Optional[bool] = False,
         time: Optional[bool] = False,
-        context: Optional[dict] = {},
+        fit_context: Optional[dict] = {},
+        application_context: Optional[dict] = {},
     ):
         """Fits this pipeline without transforming the input dataframe.
 
@@ -1009,7 +1026,10 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         time : bool, default False
             If True, per-stage application time is measured and reported when
             pipeline application is done.
-        context : dict, optional
+        fit_context : dict, option
+            Context for the entire pipeline, is retained after the pipeline
+            application is completed.
+        application_context : dict, optional
             Context to add to the application context of this call. Can map
             str keys to arbitrary object values to be used by pipeline stages
             during this pipeline application.
@@ -1025,7 +1045,8 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
             exraise=exraise,
             verbose=verbose,
             time=time,
-            context=context,
+            fit_context=fit_context,
+            application_context=application_context,
         )
         return X
 
@@ -1035,13 +1056,13 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         y: Optional[Iterable[float]] = None,
         exraise: Optional[bool] = None,
         verbose: Optional[bool] = None,
-        context: Optional[dict] = {},
+        application_context: Optional[dict] = {},
     ) -> pandas.DataFrame:
         inter_x = X
         times = []
         prev = time.time()
         self.application_context = PdpApplicationContext()
-        self.application_context.update(context)
+        self.application_context.update(application_context)
         for i, stage in enumerate(self._stages):
             try:
                 stage.fit_context = self.fit_context
@@ -1073,7 +1094,7 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         exraise: Optional[bool] = None,
         verbose: Optional[bool] = None,
         time: Optional[bool] = False,
-        context: Optional[dict] = {},
+        application_context: Optional[dict] = {},
     ) -> pandas.DataFrame:
         """Transforms the given dataframe without fitting this pipeline.
 
@@ -1100,7 +1121,7 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
         time : bool, default False
             If True, per-stage application time is measured and reported when
             pipeline application is done.
-        context : dict, optional
+        application_context : dict, optional
             Context to add to the application context of this call. Can map
             str keys to arbitrary object values to be used by pipeline stages
             during this pipeline application.
@@ -1117,12 +1138,14 @@ class PdPipeline(PdPipelineStage, collections.abc.Sequence):
                     " unfitted!").format(stage))
         if time:
             return self.__timed_transform(
-                X=X, y=y, exraise=exraise, verbose=verbose, context=context)
+                X=X, y=y, exraise=exraise, verbose=verbose,
+                application_context=application_context)
         inter_df = X
         self.application_context = PdpApplicationContext()
-        self.application_context.update(context)
+        self.application_context.update(application_context)
         for i, stage in enumerate(self._stages):
             try:
+                stage.fit_context = self.fit_context
                 stage.application_context = self.application_context
                 inter_df = stage.transform(
                     X=inter_df,


Test patch:
diff --git a/tests/core/test_app_context.py b/tests/core/test_app_context.py
index 947aa77..f6e0fb7 100644
--- a/tests/core/test_app_context.py
+++ b/tests/core/test_app_context.py
@@ -132,7 +132,7 @@ def test_application_context_injection():
     assert len(pipeline) == 2
     df = _test_df()
     val = randint(840, 921)
-    res_df = pipeline.apply(df, verbose=True, context={'a': val})
+    res_df = pipeline.apply(df, verbose=True, fit_context={'a': val})
     assert 'num1' in res_df.columns
     assert 'num1+val' in res_df.columns
     assert 'num2' in res_df.columns