Skip to content
New issue

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

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

Already on GitHub? # to your account

Progressively updated doc references (WIP) #1714

Merged
merged 4 commits into from
Jun 8, 2021

Conversation

Srijha09
Copy link
Contributor

@Srijha09 Srijha09 commented Jun 1, 2021

Description

Resolves #1188. The external reference for backend_kwargs in plot_hdi has been added. Additionally, the see also section has been extended in compare with an addition of references. The work is still in progress for the other functions.

Checklist

  • plot_hdi: nearly finished, only external references for backend_kwargs are missing.
  • compare: extend see also section, maybe use references too? plot_separation is a good example on how to format references in docstrings

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 for the PR!

Comment on lines 74 to 75
These are kwargs specific to the backend being used, passed to :meth:`mpl:matplotlib.axes.Axes.plot` or
:meth:`bokeh:bokeh.plotting.figure.Figure.patch`.
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
These are kwargs specific to the backend being used, passed to :meth:`mpl:matplotlib.axes.Axes.plot` or
:meth:`bokeh:bokeh.plotting.figure.Figure.patch`.
These are kwargs specific to the backend being used, passed to
:meth:`mpl:matplotlib.axes.Axes.plot` or
:meth:`bokeh:bokeh.plotting.figure.Figure.patch`.

pylint is complaining the line is too long: https://dev.azure.com/ArviZ/ArviZ/_build/results?buildId=4420&view=logs&j=8a326fc5-acd0-5109-b239-57a83132e152&t=1e642e4d-ea03-518d-4dda-a3ee822acd3f&l=15

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I'll make the changes

Comment on lines 163 to 164
.. [1] Using Advanced Plot Types in Kaluza: Comparison Plots for Visualization of the TCR V
Beta Repertoire, see https://www.beckman.com/flow-cytometry/software/kaluza/learning-center/comparison-plot
Copy link
Member

Choose a reason for hiding this comment

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

These are called comparison plots too but are actually not related to ours 😅. The reference should be https://doi.org/10.1007/s11222-016-9696-4. pylint is also complaining about the line length, I hope the doi link will be shorter, otherwise I don't really know how to fix that, I'd have to search and ask around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review. My bad! Sorry for the confusion. I'll change the reference. Also just to clarify, the references must be linked to a journal?

Copy link
Member

Choose a reason for hiding this comment

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

not necessarily, they can also link to books, websites, or even documentation or examples from other libraries. https://arviz-devs.github.io/arviz/api/generated/arviz.plot_loo_pit.html has one of each for example. They will often be papers though, because arviz aims to implement the latest algorithms which are generally not yet covered in books and are generally published as papers that describe them and show they work as intended.

@Srijha09 Srijha09 marked this pull request as draft June 1, 2021 17:55
@Srijha09 Srijha09 marked this pull request as ready for review June 1, 2021 17:56
@Srijha09
Copy link
Contributor Author

Srijha09 commented Jun 1, 2021

Hey, I have updated the changes. I'm still getting a trailing whitespace error although I didn't have this issue in VSCode Editor with pylint enabled. Not sure how to fix this!

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 should do it. pylint indicates the problematic line. Trailing whitespaces are often ignored so unless the config used by pylint when run via vscode is exactly the same as arviz one it is not surprising to find extra errors and warning on ci

Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
@codecov
Copy link

codecov bot commented Jun 2, 2021

Codecov Report

Merging #1714 (1dbeac8) into main (4d9caca) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1714   +/-   ##
=======================================
  Coverage   90.86%   90.86%           
=======================================
  Files         108      108           
  Lines       11821    11821           
=======================================
  Hits        10741    10741           
  Misses       1080     1080           
Impacted Files Coverage Δ
arviz/plots/hdiplot.py 92.45% <ø> (ø)
arviz/stats/stats.py 96.60% <ø> (ø)

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 4d9caca...1dbeac8. Read the comment docs.

@canyon289 canyon289 merged commit 075e07f into arviz-devs:main Jun 8, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Progressively update docs references
3 participants