Let plot_probe accept xlims, ylims and zlims independently - #462
Open
adityasingh2400 wants to merge 1 commit into
Open
Let plot_probe accept xlims, ylims and zlims independently#462adityasingh2400 wants to merge 1 commit into
adityasingh2400 wants to merge 1 commit into
Conversation
The guard that fills in missing axis limits is an OR, but its body rebound
all three names at once:
if xlims is None or ylims is None or (zlims is None and probe.ndim == 3):
xlims, ylims, zlims = get_auto_lims(probe)
So a caller passing only xlims tripped the guard on ylims being None and had
its xlims silently overwritten with the auto value. In 3D, omitting zlims
discarded both xlims and ylims. Either limit only took effect if every limit
was supplied.
Fill in just the limits that are None and leave the supplied ones alone.
Fixes SpikeInterface#119
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #119.
The guard that fills in missing axis limits is an OR, but its body rebound all three names at once:
So a caller passing only
xlimstripped the guard onylimsbeingNone, and had its ownxlimssilently overwritten with the auto value. In 3D, omittingzlimsdiscarded bothxlimsandylims. Either limit only took effect if every limit was supplied, which is what the issue reports.The fix fills in only the limits that are
Noneand leaves supplied ones alone.plot_probegrouppasses all three explicitly, so it is unaffected.Tests:
test_plot_probe_partial_limscovers x only, y only, both and neither, andtest_plot_probe_partial_lims_3dcovers xy without z and z without xy. Both assert against the real axis limits rather than the arguments.Restoring
src/probeinterface/plotting.pyfrom the base ref makes both fail, with the auto limits appearing where the caller's values should be. Both pass after. Full suite is green at 190 passed and 1 skipped.Disclosure: this change was prepared with AI assistance. I have reviewed and tested it.