Issue 128 #134
Issue 128 #134
Conversation
| jupyter: python3 | ||
| --- | ||
|
|
||
| ```{python} | ||
| import sys | ||
| sys.path.append('/Users/perdo/Documents/pega/pega-datascientist-tools/python') |
There was a problem hiding this comment.
Don't forget to remove this at some point ;)
There was a problem hiding this comment.
Maybe you can build a test that fails if such lines are still there. I do something similar for the R version, a test fails if it finds un-commented library source inclusion.
There was a problem hiding this comment.
Right yeah I think something like that would be cool. Probably in tandem with adding default linters/formatters. I'll create an issue for that, wanted tot do it anyway
| ``` | ||
|
|
||
| ```{python} | ||
| #| tags: [parameters] | ||
|
|
||
| # These parameters are overwritten when called externally | ||
|
|
||
| # TODO see if we can align with the params of the Health Check |
There was a problem hiding this comment.
What do we need for this?
There was a problem hiding this comment.
to support calling from CLI - and HC quarto updated with the same now
| ``` | ||
|
|
||
| ```{python} | ||
| #| tags: [parameters] | ||
|
|
||
| # These parameters are overwritten when called externally | ||
|
|
||
| # TODO see if we can align with the params of the Health Check | ||
|
|
||
| datafolder = os.path.expanduser("~/Downloads/tmp/") |
There was a problem hiding this comment.
Could you link this to a url location maybe?
There was a problem hiding this comment.
I meant rather than having a hardcoded url to /downloads/tmp, are the files in github? Then you can just link to the file in github directly
| ```{python} | ||
| # TODO to make generic make sure these context keys really exist, do intersect with names in the model data | ||
|
|
||
| channel_name = datamart.modelData.select(pl.concat_str(["Direction", "Channel"], separator="/")).unique().collect().item() |
There was a problem hiding this comment.
Can't you easily use your fancy .format function here?
| ``` | ||
|
|
||
| ```{python} | ||
| # TODO perhaps this should move into the pdstools plot functions "plotCumulativeLift" |
| fig.show() | ||
|
|
||
| # "Philip Mann" plot with simple red/green lift bars relative to base propensity | ||
| # TODO move below "Philip Mann" plot into a plot function (plotBinningLift) |
There was a problem hiding this comment.
Highlighting as not to forget
There was a problem hiding this comment.
The plot functions are a little daunting, I'll first get the quarto feature complete then move things around
| yaxis = dict(autorange="reversed"), | ||
| title="Propensity Lift", xaxis_title="", yaxis_title="") | ||
| # Not convinced why we need below. It should respect the binindex order but w/o this it doesnt always. | ||
| fig.update_yaxes(categoryorder='array', categoryarray=pm_plot_binning_table['BinSymbol']) |
There was a problem hiding this comment.
I think, if there's no missing bin, it tries to cast to numeric and tries to do magic with sorting. If there's a missing bin, then casting to numeric will fail and it will just keep the order
| title="Propensity Lift", xaxis_title="", yaxis_title="") | ||
| # Not convinced why we need below. It should respect the binindex order but w/o this it doesnt always. | ||
| fig.update_yaxes(categoryorder='array', categoryarray=pm_plot_binning_table['BinSymbol']) | ||
| # TODO: when there are many bins not all are always shown, figure out how to fix |
There was a problem hiding this comment.
|
Process Mining is going to get some abbreviation competition with Mr. Mann if he keeps thinking of new plots ;) |
|
Will send a new one in - most remarks left as TODOs for now |
Standalone model report complete content-wise. Remains some TODOs to clean up code, move to PDS tools library and generally make sure it is robust and the numbers compare visavis the R version.