Modernize portfolio optimizer #376
Conversation
|
Your changes were successfully integrated in the dev site, make sure to review |
There was a problem hiding this comment.
I have reviewed the PR. Before adding general comments, I wanted to ask you the status of these tasks you wanted to complete?
Submitted first update on portfolio optimizer. Some things that I still need to do:
* added better explanations for the code especially the newer ones related to creating the panel app * remove possibly redundant math when creating the graphs
Comments after review:
- It took me a little while to get what
252stood for, it turns out it's the number of trading days in a year. Make this a little clearer in the notebook please. - The first part of the example, that was already there, is quite dry. I don't think spending time improving that is really in the scope of our work, unless that's something that would be quick and easy for you to do!
- However, I think the introduction could be improved. At the moment it only talks about the financial aspect of the example, it could be extended a little to describe the HoloViz tools that are going to be used (hvplot, holoviews, panel) and how they support the analysis. It should also be mentioned a Panel dashboard is going to be created. Adding a screenshot of this app at the beginning would be nice (check out Isaiah's PR on
nyc_taxiand the start of thedashboard.ipynbnotebook). - My main feedback: the section you have ported from Panel's website is also very dry. We need to add some explanation about the app we are building and how we are building it. The
Build a <app>tutorials you have recently gone through could be a good template for this. - We should add this app as a deployment (check out
./template/anaconda-project.yamlto see how). And if we do that, we could also wrap it in a template at the very end of the notebook, so the deployed app looks good. - Some of the elements of the app could be made responsive (i.e. set up so they dynamically stretch to occupy the available space).
- The app is built using hvPlot's interactive API. It's possible we'll try to migrate to
pn.rx. I'm waiting for feedback from others on Discord on this point.
EDIT: I updated your original post with the modernizing checklist.
|
I have a suggestion about the notebook being a little dry and lacking context. Maybe you could throw the notebook content at ChatGPT and ask for it to add some brief explanations about the underlying analysis. That's just a suggestion, so feel free to ignore it if you don't feel like it. I think though that ChatGPT could do a good job at this, given this is I believe a very common kind of analysis. |
|
Your changes were successfully integrated in the dev site, make sure to review |
Is there anyone who is arguing for keeping it hvplot.interactive? pn.rx seems like the obvious thing to do, unless someone is very convincingly arguing otherwise. |
|
Was helping Jason with Also, we weren't sure how to separate out the widgets to better lay out the dashboard, unlike pn.Param. I'm -1 on using from io import BytesIO
import pandas as pd
import panel as pn
pn.extension()
@pn.cache
def get_stocks(data):
if data is None:
stock_file = 'https://datasets.holoviz.org/stocks/v1/stocks.csv'
else:
stock_file = BytesIO(data)
return pd.read_csv(stock_file, index_col='Date', parse_dates=True)
file_input = pn.widgets.FileInput(sizing_mode='stretch_width')
stocks = pn.rx(get_stocks)(file_input)
selector = pn.widgets.MultiSelect(
name='Select stocks', sizing_mode='stretch_width',
options=stocks.columns.to_list()
)
selected_stocks = stocks.rx.pipe(
lambda df, cols: df[cols] if cols else df, selector
)
pn.Column(selected_stocks)
Lastly, upon printing rx objects, it raises an error |
|
Ok I just realized there's a section in Panel about rx; I was too focused on the param docs. To split the widget + column, you need to wrap it in the corresponding pane; personally I'd prefer a method to call to do so, e.g. from io import BytesIO
import pandas as pd
import panel as pn
pn.extension()
@pn.cache
def get_stocks(data):
if data is None:
stock_file = 'https://datasets.holoviz.org/stocks/v1/stocks.csv'
else:
stock_file = BytesIO(data)
return pd.read_csv(stock_file, index_col='Date', parse_dates=True)
file_input = pn.widgets.FileInput(sizing_mode='stretch_width')
stocks = pn.rx(get_stocks)(file_input)
selector = pn.widgets.MultiSelect(
name='Select stocks', sizing_mode='stretch_width',
options=stocks.columns.to_list()
)
selected_stocks = stocks.rx.pipe(
lambda df, cols: df[cols] if cols else df, selector
)
pn.Column(
pn.Row(file_input, selector),
pn.pane.DataFrame(selected_stocks),
) |
|
Your changes were successfully integrated in the dev site, make sure to review |
|
Your changes were successfully integrated in the dev site, make sure to review |
Can you please make a PR on Param putting in an explicit link to the Panel rx docs wherever you would have found them? Right now it only links to the main Panel site, but pointing directly to Panel rx info would presumably prevent others from having the same experience you did. |
|
Your changes were successfully integrated in the dev site, make sure to review |
|
Your changes were successfully integrated in the dev site, make sure to review |
|
Your changes were successfully integrated in the dev site, make sure to review |
|
I made a lot of changes. There were still many issues: headings, grammatical, organizational, textual. In the future, the PRs need to be further along prior to asking for a final review. |
|
Your changes were successfully integrated in the dev site, make sure to review |
* added panel dashboard * clear outputs * added more text description to cells * improved the introduction of the notebook * removed param.watch * cleared outputs * added more cell descriptions * fixed linter error * updated holoview plots to hvplot * removed outputs * updated yml * minor edits to made code more readable * changes to cell descriptions * updated yml files * updated yaml files again * dr revision of text code and viz * color the distribution plots --------- Co-authored-by: jtao1 <jtao@LAPTOP-UKS0PRO9> Co-authored-by: Demetris Roumis <roumis.d@gmail.com>
Modernizing an example checklist
Preliminary checks
Change ‘anaconda-project.yml’ to use the latest workable version of packages
hvplot<0.9tohvplot,panel>=0.12,<1.0topanel>=0.12) of all other dependencies. Removing the upper pins of dependencies could necessitate code revisions in the notebooks to address any errors encountered in the updated environment. Should complexities or extensive time requirements arise, document issues for team discussion on whether to re-pin specific packages or explore other solutions.hvplottohvplot>=0.9.2,hvplot>=0.8tohvplot>=0.9.2). Usually, the new/updated lower pin of a dependency will be the version resolved afteranaconda preparehas been run. Execute!conda listin a notebook, oranaconda run conda listin the terminal, to display the version of each dependency installed in the environment. Adjusting the lower pin helps ensure that the locks produced for each platform (linux-64, win-64, osx-64, osx-arm64) rely on the tested dependencies and not on some older versions.Plot API updates (discussed on a per-example basis)
datashadewithrasterize(read this page). Essentially,rasterizeallows Bokeh to handle the colormapping instead of Datashader.Interactivity API updates (discussed on a per-example basis)
pn.interactusage.param.watch()usage. This is pretty low-level and verbose approach and should not be used in Examples unless required, or an Example is specifically trying to demo its usage in an advanced workflow.pn.bind(). Read this page for explanation.view()method and call it directly, update the class by inheriting frompn.viewable.Viewerand replaceview()by__panel__(). Here is an example.Panel App updates (discussed on a per-example basis)
pn.Column, or more complicated to incorporate widgets, etc. Make the final app.servable().command: dashboarddeclaration in theanaconda-project.ymlfile), try adding it.template = pn.template.BootstrampTemplate, but if building up an app across multiple cells, it is probably cleaner to declare the template at the top withpn.extension(template='bootstrap'). See how to guide on setting a template.General code quality updates
warnings.simplefilter(‘ignore’)somewhere at the start of the notebook, remove this line. Try to update the code to remove the warnings, if any. If updating the code to remove the warnings is taking significant amount of time and effort, bring it up for discussion and we may decide to disable warnings again.Text content
Visual appearance - Example
Visual appearance - Gallery
Ml AnnotatorstoML Annotators), if not, add/update theexamples_config.titlefield inanaconda-project.ymldescriptionfield inanaconda-project.ymlWorkflow (after you have made the changes above)
doit validate:<projectname>doit test:<projectname>doit doc_one –name <projectname>. It’s better if the project notebook(s) is saved with its outputs (but be sure to clear outputs before committing to the examples repo!) when building the docs. Then open this file in your browser./builtdocs/index.htmland check how the site looks.