Doc change the default value of pymc.draw from 500 to 1. #5543

purna135 · 2022-03-04T12:00:44Z

Changed the default value of draws from 500 to 1.

codecov · 2022-03-04T12:07:14Z

Codecov Report

Merging #5543 (9c6b5de) into main (4223b6e) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #5543      +/-   ##
==========================================
+ Coverage   87.24%   87.26%   +0.02%     
==========================================
  Files          81       81              
  Lines       14244    14244              
==========================================
+ Hits        12427    12430       +3     
+ Misses       1817     1814       -3

Impacted Files	Coverage Δ
pymc/sampling.py	`87.24% <ø> (ø)`
pymc/parallel_sampling.py	`87.70% <0.00%> (+0.99%)`	⬆️

OriolAbril · 2022-03-05T03:44:39Z

do you want to update the rest of the docstring following #5459? Both yes or no are fine, they just influence the scope of the review

purna135 · 2022-03-05T04:34:21Z

Yes @OriolAbril, I'm happy to update the remainder of the docstring, but I'm not sure how to set the proper type for vars, what is default for mode as it's a optional.
Could you kindly guide me how to find the default value of a variable?
I will work on all the methords present in website following #5459.

OriolAbril

the return section also needs some updates following https://pymc-data-umbrella.xyz/en/latest/sprint/docstring_tutorial.html#returns-and-yields (or what is the same https://numpydoc.readthedocs.io/en/latest/format.html#returns)

OriolAbril · 2022-03-05T04:50:02Z

pymc/sampling.py

@@ -2128,7 +2128,7 @@ def draw(
    vars


Suggested change

vars

vars : tensor_like or iterable of tensor_like

vars is missing the type but it is required, so it should have no optional nor default after the type.

OriolAbril · 2022-03-05T04:52:21Z

pymc/sampling.py

@@ -2128,7 +2128,7 @@ def draw(
    vars
        A variable or a list of variables for which to draw samples.
    draws : int


Suggested change

draws : int

draws : int, default 1

draws is optional, so it should have either optional or default. As here the default is of the documented type and a specific value, using default instead of optional is preferred, as explained in the third point of https://pymc-data-umbrella.xyz/en/latest/sprint/docstring_tutorial.html#parameters. The default would then be removed from the description below

OriolAbril · 2022-03-05T04:57:09Z

pymc/sampling.py

@@ -2128,7 +2128,7 @@ def draw(
    vars
        A variable or a list of variables for which to draw samples.
    draws : int
-        Number of samples needed to draw. Detaults to 500.
+        Number of samples needed to draw. Detaults to 1.
    mode


this is missing the type and the , optional

OriolAbril · 2022-03-05T04:57:26Z

pymc/sampling.py

@@ -2128,7 +2128,7 @@ def draw(
    vars
        A variable or a list of variables for which to draw samples.
    draws : int
-        Number of samples needed to draw. Detaults to 500.
+        Number of samples needed to draw. Detaults to 1.
    mode
        The mode used by ``aesara.function`` to compile the graph.
    **kwargs


Suggested change

**kwargs

**kwargs : dict, optional

Thank you, I will follow the same : )

purna135 · 2022-03-05T06:21:24Z

pymc/sampling.py

    Returns
    -------
-    List[np.ndarray]
+    out : list of ndarrays
        A list of numpy arrays.


I'm not sure if this return statement is correct; please have a look.

OriolAbril · 2022-03-05T06:57:02Z

pymc/sampling.py

@@ -2125,18 +2125,18 @@ def draw(

    Parameters
    ----------
-    vars
+    vars : tensor_like or iterable of tensor_like


I made a mistake here before, tensor_like includes things that can be converted to aesara tensorvariable like lists or numpy arrays, that is not the case here.

Suggested change

vars : tensor_like or iterable of tensor_like

vars : Variable or iterable of Variable

OriolAbril · 2022-03-05T06:59:18Z

pymc/sampling.py

-    **kwargs
+    draws : int, default 1
+        Number of samples needed to draw.
+    mode : str or `Mode` instance, optional


there should be no backticks in type descriptions. I am not sure if Mode is a common input type, if it is, this should be Mode without backticks nor instance and we'll add a numpydoc alias. If it is not, it should be aesara.compile.mode.Mode again no backticks nor instance.

OriolAbril · 2022-03-05T07:01:18Z

pymc/sampling.py

        Keyword arguments for :func:`pymc.aesara.compile_pymc`

    Returns
    -------
-    List[np.ndarray]
+    out : list of ndarrays


Suggested change

out : list of ndarrays

list of ndarray

types should always be used in singular version, and in numpydoc, if there is only one output it should not be named, only the type info should be present.

OriolAbril · 2022-03-05T07:02:11Z

pymc/sampling.py

+    draws : int, default 1
+        Number of samples needed to draw.
+    mode : str or `Mode` instance, optional
+        The mode used by :func:``aesara.function`` to compile the graph.


Suggested change

The mode used by :func:``aesara.function`` to compile the graph.

The mode used by :func:`aesara.function` to compile the graph.

purna135 · 2022-03-05T07:16:41Z

Thank you very much, @OriolAbril; I'm learning a lot from you 😊😃

pymc/sampling.py

changed the default value of drwas from 500 to 1

193883d

ricardoV94 approved these changes Mar 4, 2022

View reviewed changes

OriolAbril reviewed Mar 5, 2022

View reviewed changes

OriolAbril mentioned this pull request Mar 5, 2022

Review and update API docs #5282

Open

fixed samplers_draw docsting

bc53f6c

purna135 commented Mar 5, 2022

View reviewed changes

OriolAbril reviewed Mar 5, 2022

View reviewed changes

fixed samplers_draw docsting

9c6b5de

purna135 commented Mar 5, 2022

View reviewed changes

pymc/sampling.py Show resolved Hide resolved

OriolAbril approved these changes Mar 6, 2022

View reviewed changes

OriolAbril merged commit 620546b into pymc-devs:main Mar 6, 2022

purna135 deleted the doc_samplers_draw branch March 12, 2022 10:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Doc change the default value of pymc.draw from 500 to 1. #5543

Doc change the default value of pymc.draw from 500 to 1. #5543

purna135 commented Mar 4, 2022

codecov bot commented Mar 4, 2022 •

edited

Loading

OriolAbril commented Mar 5, 2022

purna135 commented Mar 5, 2022

OriolAbril left a comment

OriolAbril Mar 5, 2022

OriolAbril Mar 5, 2022

OriolAbril Mar 5, 2022

OriolAbril Mar 5, 2022

purna135 Mar 5, 2022

purna135 Mar 5, 2022

OriolAbril Mar 5, 2022

OriolAbril Mar 5, 2022

OriolAbril Mar 5, 2022

OriolAbril Mar 5, 2022

purna135 commented Mar 5, 2022

	vars : tensor_like or iterable of tensor_like
	vars : Variable or iterable of Variable

	The mode used by :func:``aesara.function`` to compile the graph.
	The mode used by :func:`aesara.function` to compile the graph.

Doc change the default value of pymc.draw from 500 to 1. #5543

Doc change the default value of pymc.draw from 500 to 1. #5543

Conversation

purna135 commented Mar 4, 2022

codecov bot commented Mar 4, 2022 • edited Loading

Codecov Report

OriolAbril commented Mar 5, 2022

purna135 commented Mar 5, 2022

OriolAbril left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purna135 commented Mar 5, 2022

codecov bot commented Mar 4, 2022 •

edited

Loading