DOC Added example comparing L1-based models to ARD user guide#31425
DOC Added example comparing L1-based models to ARD user guide#31425adrinjalali merged 19 commits intoscikit-learn:mainfrom
Conversation
…ted example comparing Bayesian Regressors into text
…egrated example comparing Bayesian Regressors into text
StefanieSenger
left a comment
There was a problem hiding this comment.
Thank you for taking this back up, @supersom!
If integration into text is the preferred model, should I remove the 'example' section for ARD?
Yes, you definitely should remove the examples section in doc/modules/linear_model.rst (li. 845-849 currently), since this contains one of the two examples you have linked above and we don't want to have duplicated links.
I would like to finish on these two links, since it seems that #30587 is stalled.
Apart from this, sphx_glr_auto_examples_linear_model_plot_lasso_and_elasticnet.py is already linked sufficiently in the user guide, I belive, and sphx_glr_auto_examples_linear_model_plot_ard.py is already linked in ARDRegressor's docstring. Is there something from your perspective that, were one of the two links need to be linked?
…ARD docstrings and demonstrating ARD to ARD docstring [as in PR scikit-learn#30587]
…nto doc_link_example
|
Thanks for clarifying, @StefanieSenger!
I'm not sure I followed you on this one - could you help me out please? |
|
Thank you for your further work, @supersom.
Oh I see I did not express myself well, sorry about that. I meant that I don't think the links from the other PR are really necessary, and asked for your opinion. According to CI there are also several places with double line breaks (we are using numpydoc style guide): Could you please fix these? |
|
Thanks for the review, @StefanieSenger!
Crystal clear now - thank you!
Sure - I'm on it. New to reST and figuring out local testing - so it's taking a few turns. |
StefanieSenger
left a comment
There was a problem hiding this comment.
Thanks for fixing the rendering issues, @supersom.
Now, two examples appear twice, and we need to remove the duplicates.
I am not so convinced about the links in the docstrings, since I think general examples like these are more a matter for the user guide, and we want to keep examples section in the API reference as clean as possible and they should be for pure code examples only.
But I will leave this for a maintainer to decide on.
doc/modules/linear_model.rst
Outdated
| See :ref:`sphx_glr_auto_examples_linear_model_plot_ard.py` for a worked-out comparison between ARD and `Bayesian Ridge Regression`_. | ||
|
|
||
| See :ref:`sphx_glr_auto_examples_linear_model_plot_lasso_and_elasticnet.py` for a comparison between various methods - Lasso, ARD and ElasticNet - on correlated data. | ||
|
|
There was a problem hiding this comment.
| See :ref:`sphx_glr_auto_examples_linear_model_plot_ard.py` for a worked-out comparison between ARD and `Bayesian Ridge Regression`_. | |
| See :ref:`sphx_glr_auto_examples_linear_model_plot_lasso_and_elasticnet.py` for a comparison between various methods - Lasso, ARD and ElasticNet - on correlated data. |
Now these two are linked twice. So, we should remove the duplicates. (The line after should also be removed, to avoid double blank lines again, but I cannot comment there.)
There was a problem hiding this comment.
Now these two are linked twice.
Thank you so much for pointing that out, @StefanieSenger - I overlooked auto-merge getting too helpful! Should be fixed now.
we want to keep examples section in the API reference as clean as possible and they should be for pure code examples only.
That sounds like a reasonable plan - code examples in docstrings and general examples in user guides. Since #30587 was already underway, I just brought those changes over TBH.
StefanieSenger
left a comment
There was a problem hiding this comment.
Thanks for your most recent adjustments, @supersom.
Now everything is in good shape. Only the question on whether to add these examples to the API reference of ElasticNet, Lasso and ARDRegressor is still open. What would you think, @adrinjalali?
Hi @adrinjalali , @StefanieSenger - Please let me know how we could take this to a merge. |
Reference Issues/PRs
Towards #30621
What does this implement/fix? Explain your changes.
Any other comments?
If integration into text is the preferred model, should I remove the 'example' section for ARD?