Skip to content

Conversation

@martinkilbinger
Copy link
Contributor

@martinkilbinger martinkilbinger commented Oct 9, 2025

Compute various catalogue quantities binned in SNR and size, for illustration and testing.

@martinkilbinger martinkilbinger changed the title Binned Binned quantities Oct 9, 2025
@martinkilbinger martinkilbinger self-assigned this Oct 9, 2025
@martinkilbinger martinkilbinger added the enhancement New feature or request label Oct 9, 2025
@martinkilbinger martinkilbinger requested review from cailmdaley and removed request for sachaguer January 20, 2026 13:50
Copy link
Collaborator

@cailmdaley cailmdaley left a comment

Choose a reason for hiding this comment

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

Review: Two critical issues to fix

Thanks for extending the binned quantities functionality! Found two issues that will prevent the notebook from running:

1. Stray s on line 143 of leakage_minimal.py

# Line 143-144
s
quantities, bin_edges = calibration.get_quantities_binned(...)

This will raise a NameError. Should be removed.

2. Undefined variable which in leakage_minimal.py:144

quantities, bin_edges = calibration.get_quantities_binned(cat_gal, num_bins_x, num_bins_y, which=which)

The variable which is never defined in the notebook. Probably needs something like:

which = ["number", "response", "leakage", "w_iv", "mag", ...]

Minor items (non-blocking):

  • Duplicate import numpy as np in plot_binned_quantities.py:16
  • Commented-out matplotlib inline block could be cleaned up

Otherwise looks good! 👍

cailmdaley

This comment was marked as duplicate.

cailmdaley

This comment was marked as duplicate.

Copy link
Collaborator

@cailmdaley cailmdaley left a comment

Choose a reason for hiding this comment

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

Review: Two critical issues to fix

Thanks for extending the binned quantities functionality! Found two issues that will prevent the notebook from running:

1. Stray s on line 143 of leakage_minimal.py

This will raise a NameError. Should be removed.

2. Undefined variable which in leakage_minimal.py:144

The variable which is never defined in the notebook. Probably needs to be defined before calling get_quantities_binned().


Minor items (non-blocking):

  • Duplicate import numpy as np in plot_binned_quantities.py:16
  • Commented-out matplotlib inline block could be cleaned up

Otherwise looks good!

@martinkilbinger
Copy link
Contributor Author

Bugs fixed.

@cailmdaley cailmdaley self-requested a review January 27, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants