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

ENH: add functionality symlog to numpy.histogram #26287

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MycrofD
Copy link

@MycrofD MycrofD commented Apr 16, 2024

This PR introduces a new parameter called symlog to numpy.histogram.
Details are in the issue raised at #24368.

Briefly, it gives geometrically spaced bins for data containing positive and negative values when an integer number of bins are asked for.

@MycrofD MycrofD force-pushed the feat-histograms-symlogbins branch 3 times, most recently from 1d2108f to 3721747 Compare April 16, 2024 10:53
@MycrofD MycrofD changed the title ENH: add functionality symlog to numpy.histogram, see #24368 ENH: add functionality symlog to numpy.histogram Apr 16, 2024
@MycrofD MycrofD changed the title ENH: add functionality symlog to numpy.histogram ENH: add functionality symlog to numpy.histogram Apr 16, 2024
Copy link
Contributor

@eendebakpt eendebakpt left a comment

Choose a reason for hiding this comment

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

A few review comments. The main question for me would be: should we include an option for this? A user can always calculate the bins and pass that to np.histogram. Is this used often enough to warrant an extra flag.

Compute the bin edges for a histogram with geometrically spaced bins.
The bins are spaced such that the width of each bin is constant in
log-space.
Reference issue: https://github.com/numpy/numpy/issues/24368
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Reference issue: https://github.com/numpy/numpy/issues/24368

No need to include references to github issues for user documentation

@@ -383,6 +383,17 @@ def _get_bin_edges(a, bins, range, weights):
# parse the overloaded bins argument
n_equal_bins = None
bin_edges = None
if symlog:
if np.ndim(bins) != 0:
warnings.warn(
Copy link
Contributor

Choose a reason for hiding this comment

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

This should raise an error

Comment on lines +394 to +395
n_unequal_bins = bins
bin_edges = _get_geomspace_edges(n_unequal_bins, a)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
n_unequal_bins = bins
bin_edges = _get_geomspace_edges(n_unequal_bins, a)
bin_edges = _get_geomspace_edges(bins, a)

@@ -453,6 +464,37 @@ def _get_bin_edges(a, bins, range, weights):
return bin_edges, None


def _get_geomspace_edges(n_unequal_bins, a):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def _get_geomspace_edges(n_unequal_bins, a):
def _get_geomspace_edges(a, n_unequal_bins):

(keep the same order as _get_bin_edges)

"""
# The idea is to use the absolute min and max of the data to compute the
# range, and then the pseudo-first and last edge.
pseudo_first_edge, pseudo_last_edge = abs(a).min(), abs(a).max()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pseudo_first_edge, pseudo_last_edge = abs(a).min(), abs(a).max()
abs_a = abs(a)
pseudo_first_edge, pseudo_last_edge = abs_a.min(), abs_a.max()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants