8000 DOC: random: fix doc linking, was referencing private submodules. by rgommers · Pull Request #14360 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

DOC: random: fix doc linking, was referencing private submodules. #14360

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

Merged
merged 2 commits into from
Sep 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions doc/source/reference/random/bit_generators/mt19937.rst
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
Mersenne Twister (MT19937)
Mersenne Twister (MT19937)
--------------------------

.. module:: numpy.random.mt19937

.. currentmodule:: numpy.random.mt19937
.. currentmodule:: numpy.random
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure current module needs to change? Doc render looks right. The module isn't private (numpy.random.mt19937).

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment for the other bit generators.

Copy link
Member Author

Choose a reason for hiding this comment

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

The module isn't private

unfortunately we have never used underscores correctly (or much at all), which I think is what you're referring to as "private". but that's definitely a private submodule.

Copy link
Contributor

Choose a reason for hiding this comment

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

It wasn't my intent to make it private. There are use cases where someone wants to consuma a bit generator but doesn't want anything else (.e.g writing their own low-level Generator-like objects). Should they be directed to numpy.random.init. Also, there are a lot of intentionally private parts to random, so I think there was some consideration about what should and shouldn't be private here (or what should be stable and what is not).

Copy link
Member Author

Choose a reason for hiding this comment

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

There's a couple of issues with that:

  • We shouldn't have one object in two public places (np.random.MT19937 is np.random.mt19937.MT19937)
  • Deep nesting isn't great (see import this - flat is better than nested)
  • Public submodules with a single object in them don't name too much sense as API design.

Also, this was never discussed I believe. You can't just add many new submodules to the numpy API without any proposal or discussion. My understanding was that all public functions and classes were added to the numpy.random namespace. Hence my question in the summary of this PR about the few functions that are missing there - were they intended to be public or not?

Should they be directed to numpy.random.__init__.

I don't see a problem with that. np.random.MT19937 is clearer than np.random.mt19937.MT19937.


.. autoclass:: MT19937
:exclude-members:
Expand Down
4 changes: 1 addition & 3 deletions doc/source/reference/random/bit_generators/pcg64.rst
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
Parallel Congruent Generator (64-bit, PCG64)
--------------------------------------------

.. module:: numpy.random.pcg64

.. currentmodule:: numpy.random.pcg64
.. currentmodule:: numpy.random

.. autoclass:: PCG64
:exclude-members:
Expand Down
4 changes: 1 addition & 3 deletions doc/source/reference/random/bit_generators/philox.rst
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
Philox Counter-based RNG
------------------------

.. module:: numpy.random.philox

.. currentmodule:: numpy.random.philox
.. currentmodule:: numpy.random

< 8000 /td> .. autoclass:: Philox
:exclude-members:
Expand Down
4 changes: 1 addition & 3 deletions doc/source/reference/random/bit_generators/sfc64.rst
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
SFC64 Small Fast Chaotic PRNG
-----------------------------

.. module:: numpy.random.sfc64

.. currentmodule:: numpy.random.sfc64
.. currentmodule:: numpy.random

.. autoclass:: SFC64
:exclude-members:
Expand Down
2 changes: 1 addition & 1 deletion doc/source/reference/random/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ Concepts
:maxdepth: 1

generator
legacy mtrand <legacy>
Legacy Generator (RandomState) <legacy>
BitGenerators, SeedSequences <bit_generators/index>

Features
Expand Down
18 changes: 8 additions & 10 deletions doc/source/reference/random/legacy.rst
628C
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,27 @@

Legacy Random Generation
------------------------
The `~mtrand.RandomState` provides access to
The `RandomState` provides access to
legacy generators. This generator is considered frozen and will have
no further improvements. It is guaranteed to produce the same values
as the final point release of NumPy v1.16. These all depend on Box-Muller
normals or inverse CDF exponentials or gammas. This class should only be used
if it is essential to have randoms that are identical to what
would have been produced by previous versions of NumPy.

`~mtrand.RandomState` adds additional information
`RandomState` adds additional information
to the state which is required when using Box-Muller normals since these
are produced in pairs. It is important to use
`~mtrand.RandomState.get_state`, and not the underlying bit generators
`RandomState.get_state`, and not the underlying bit generators
`state`, when accessing the state so that these extra values are saved.

Although we provide the `~mt19937.MT19937` BitGenerator for use independent of
`~mtrand.RandomState`, note that its default seeding uses `~SeedSequence`
rather than the legacy seeding algorithm. `~mtrand.RandomState` will use the
Although we provide the `MT19937` BitGenerator for use independent of
`RandomState`, note that its default seeding uses `SeedSequence`
rather than the legacy seeding algorithm. `RandomState` will use the
legacy seeding algorithm. The methods to use the legacy seeding algorithm are
currently private as the main reason to use them is just to implement
`~mtrand.RandomState`. However, one can reset the state of `~mt19937.MT19937`
using the state of the `~mtrand.RandomState`:
`RandomState`. However, one can reset the state of `MT19937`
using the state of the `RandomState`:

.. code-block:: python

Expand All @@ -47,8 +47,6 @@ using the state of the `~mtrand.RandomState`:
rs2.standard_exponential()


.. currentmodule:: numpy.random.mtrand

.. autoclass:: RandomState
:exclude-members:

Expand Down
4 changes: 2 additions & 2 deletions numpy/random/mtrand.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ cdef class RandomState:
See Also
--------
Generator
mt19937.MT19937
Bit_Generators
MT19937
:ref:`bit_generator`

"""
cdef public object _bit_generator
Expand Down
0