Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add ArviZ installation guide [docs] #1551

Merged
merged 5 commits into from
Mar 1, 2021

Conversation

arungrace88
Copy link
Contributor

Description

This is an interim update on issue 'Write installation guide #1487' for your perusal. The .rst file contains the guide to install ArviZ using either pip or conda forge. The required and optional dependencies are also discussed in the guide. Kindly have a look and let me know the feedback. Thanks.

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you for the contribution. I have added several comments, to try and start some discussion and back and forth feedback, they are not must do changes.

I also wanted to note a couple extra points. First is that I would place the document inside the getting started folder, and for the file to actually be shown in docs, it has to be added at the toctree in https://github.com/arviz-devs/arviz/blob/main/doc/source/getting_started/index.md (I would put it in the first place).

Second extra comment is that I think that one can install all optional dependencies with pip install arviz[all] thanks to https://github.com/arviz-devs/arviz/blob/main/setup.py#L57, can you check and if it works add that to the guide? Not sure if in pip section or if in dependencies section though

I have also added [docs] to the PR title to have Azure generate the docs so we have a preview. They can be download from Azure as artifacts.

doc/source/installation.rst Outdated Show resolved Hide resolved
doc/source/installation.rst Outdated Show resolved Hide resolved
doc/source/installation.rst Outdated Show resolved Hide resolved
doc/source/installation.rst Outdated Show resolved Hide resolved
doc/source/installation.rst Outdated Show resolved Hide resolved
doc/source/installation.rst Outdated Show resolved Hide resolved
@OriolAbril OriolAbril changed the title [WIP] Add ArviZ installation guide [WIP] Add ArviZ installation guide [docs] Feb 10, 2021
@codecov
Copy link

codecov bot commented Feb 16, 2021

Codecov Report

Merging #1551 (8af60c0) into main (bceb7e1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1551   +/-   ##
=======================================
  Coverage   90.05%   90.05%           
=======================================
  Files         108      108           
  Lines       11584    11584           
=======================================
  Hits        10432    10432           
  Misses       1152     1152           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bceb7e1...b3f1658. Read the comment docs.

@arungrace88
Copy link
Contributor Author

@OriolAbril,
I have updated the Installation.rst file and index.md file in the getting started folder. Double checked the command pip install arviz[all], and it installs all the optional dependencies. This is documented in the installation page. Kindly have a look. Thanks.

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It looks great

Before merging, I would add a link to this page in the home page. It currently has pip, conda and dev installation instrucctions, I think it would be great to leave pip and conda and change the dev install with something like "See the installation guide for more details"

functionality is available with the basic requirements, but ArviZ also has optional
dependencies to further enhance the library. This guide will cover both basic and fully-fledged ArviZ installs and several installation methods.

ArviZ can be installed either using pip or conda-forge
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put this line a couple lines later, inside the stable section

Comment on lines 46 to 56
ArviZ's functions work with NumPy arrays, dictionaries of arrays, xarray datasets, and has built-in support for `PyMC3 <https://docs.pymc.io/>`_,
`PyStan <https://pystan.readthedocs.io/en/latest/>`_, `CmdStanPy <https://github.com/stan-dev/cmdstanpy>`_,
`Pyro <http://pyro.ai/>`_, `NumPyro <http://num.pyro.ai/>`_,
`emcee <https://emcee.readthedocs.io/en/stable/>`_, and
`TensorFlow Probability <https://www.tensorflow.org/probability>`_ objects. Support for PyMC4, Edward2, and Edward are on the roadmap.

A Julia wrapper, `ArviZ.jl <https://arviz-devs.github.io/ArviZ.jl/stable/>`_ is
also available. It provides built-in support for
`Turing.jl <https://turing.ml/dev/>`_, `CmdStan.jl
<https://github.com/StanJulia/CmdStan.jl>`_, `StanSample.jl
<https://github.com/StanJulia/StanSample.jl>`_ and `Stan.jl <https://github.com/StanJulia/Stan.jl>`_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this.


- UltraJSON

If available, ArviZ makes use of faster ujson when ``arviz.from_json(filename)`` is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If available, ArviZ makes use of faster ujson when ``arviz.from_json(filename)`` is
If available, ArviZ makes use of faster ujson when :func:`arviz.from_json` is

Writing the function like this will add a hyperlink to the corresponding page in the api reference

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Thanks for letting me know.

@@ -0,0 +1,100 @@
############
Installation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Installation
Installation guide

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

@arungrace88
Copy link
Contributor Author

Hi @OriolAbril, Added changes to index and installation file. Kindly have a look. Thanks.

@OriolAbril
Copy link
Member

This looks great, thanks @arungrace88 ! I will merge it once it is added to the changelog.

@OriolAbril OriolAbril merged commit db61a6d into arviz-devs:main Mar 1, 2021
@OriolAbril
Copy link
Member

I took the liberty of updating the changelog myself and merged, thanks for the PR @arungrace88

utkarsh-maheshwari pushed a commit to utkarsh-maheshwari/arviz that referenced this pull request May 27, 2021
* ArviZ installation page for review

* Updated Installation.rst and index.md file in Getting Started documentation page

* Deleted the previous installation.rst file in the source folder

* Add link to Installation guide from home page. Update to Installation guide.

* update changelog

Co-authored-by: Oriol (ZBook) <oriol.abril.pla@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants