8000 MAINT: backport Cython API cleanup to 1.17.x, remove docs by rgommers · Pull Request #14593 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

MAINT: backport Cython API cleanup to 1.17.x, remove docs #14593

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 4 commits into from
Nov 19, 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
4 changes: 4 additions & 0 deletions doc/release/1.17.3-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ Compatibility notes
===================

- The seldom used ``PyArray_DescrCheck`` macro has been changed/fixed.
- The use of the new ``numpy.random`` features from Cython and Numba
was not well documented and parts have been removed or refactored.
We plan to finish the refactor and fully document it in 1.18.0



Contributors
Expand Down
165 changes: 0 additions & 165 deletions doc/source/reference/random/extending.rst

This file was deleted.

5 changes: 0 additions & 5 deletions doc/source/reference/random/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,6 @@ What's New or Different
* Optional ``out`` argument that allows existing arrays to be filled for
select distributions
* All BitGenerators can produce doubles, uint64s and uint32s via CTypes
Copy link
Contributor

Choose a reason for hiding this comment

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

This statement is still true on 1.17.x.

Copy link
Member

Choose a reason for hiding this comment

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

@rgommers Comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

no idea, I have no reason to doubt @bashtage's comment

Copy link
Member

Choose a reason for hiding this comment

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

Should we omit the deletion here, or is reasonable for 1.17 and we will cover things better in 1.18? I'd like to put this backport in. Maybe have another if there are changes in master?

Copy link
Member Author

Choose a reason for hiding this comment

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

Happy to omit this deletion, will verify later today or tomorrow and change if it still works.

Copy link
Member

Choose a reason for hiding this comment

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

OK, will wait. @bashtage makes the same comment in several places.

Copy link
Member

Choose a reason for hiding this comment

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

@rgommers Still want to look at this?

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry, failed to make time - not going to happen until end of next week at the earliest. added the 1.17.4 milestone, it seems that it should go into that release

Copy link
Member

Choose a reason for hiding this comment

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

reverted the deletion: BitGenerator.ctypes still exists

(`~.PCG64.ctypes`) and CFFI (`~.PCG64.cffi`). This allows the bit generators
to be used in numba.
* The bit generators can be used in downstream projects via
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically this statement is also True, since bit generator all export a capsule that can be unpacked in Cython get
the function pointers.

Copy link
Member

Choose a reason for hiding this comment

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

reverted the deletion

Copy link
Member

Choose a reason for hiding this comment

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

on second thought, I think that use is too obscure to be practical. Let's remove that from this release.

:ref:`Cython <randomgen_cython>`.
* `~.Generator.integers` is now the canonical way to generate integer
random numbers from a discrete uniform distribution. The ``rand`` and
``randn`` methods are only available through the legacy `~.RandomState`.
Expand Down Expand Up @@ -199,7 +195,6 @@ Features
Multithreaded Generation <multithreading>
new-or-different
Comparing Performance <performance>
extending

Original Source
~~~~~~~~~~~~~~~
Expand Down
7 changes: 3 additions & 4 deletions doc/source/reference/random/new-or-different.rst
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,12 @@ And in more detail:
``randn`` methods are only available through the legacy `~.RandomState`.
This replaces both ``randint`` and the deprecated ``random_integers``.
* The Box-Muller method used to produce NumPy's normals is no longer available.
* All bit generators can produce doubles, uint64s and
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.

Copy link
Member

Choose a reason for hiding this comment

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

reverted

uint32s via CTypes (`~PCG64.ctypes`) and CFFI (`~PCG64.cffi`).
This allows these bit generators to be used in numba.
* All bit generators can produce doubles, uint64s and uint32s via CTypes
(`~PCG64.ctypes`) and CFFI (`~PCG64.cffi`). This allows these bit generators
to be used in numba.
* The bit generators can be used in downstream projects via
Cython.


.. ipython:: python

from numpy.random import Generator, PCG64
Expand Down
2 changes: 1 addition & 1 deletion numpy/random/common.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ from libc.stdint cimport (uint8_t, uint16_t, uint32_t, uint64_t,
uintptr_t)
from libc.math cimport sqrt

cdef extern from "numpy/random/bitgen.h":
cdef extern from "src/bitgen.h":
Copy link
Member

Choose a reason for hiding this comment

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

The original is what is in 1.17 and will also be in 1.18 after the C-API changes go in.

struct bitgen:
void *state
uint64_t (*next_uint64)(void *st) nogil
Expand Down
2 changes: 0 additions & 2 deletions numpy/random/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ def generate_libraries(ext, build_dir):

defs.append(('NPY_NO_DEPRECATED_API', 0))
config.add_data_dir('tests')
config.add_d F438 ata_files('common.pxd')
config.add_data_files('bit_generator.pxd')

EXTRA_LINK_ARGS = []
# Math lib
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion numpy/random/src/distributions/distributions.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "Python.h"
#include "numpy/npy_common.h"
#include "numpy/npy_math.h"
#include "numpy/random/bitgen.h"
#include "src/bitgen.h"
Copy link
Member

Choose a reason for hiding this comment

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

This isn't correct for 1.17, and it looks like 1.17 will be the same as 1.18 after the C-API changes go in.

charris@fc [numpy.git ((v1.17.4))]$ find . -name 'bitgen\.h'
./numpy/core/include/numpy/random/bitgen.h

Copy link
Member
@charris charris Nov 18, 2019

Choose a reason for hiding this comment

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

The current 1.17 file is renamed in this PR, do we want to do that for an existing release?


/*
* RAND_INT_TYPE is used to share integer generators with RandomState which
Expand Down
0