Skip to content

let user retrieve the location of the dip#77

Closed
JohannesBuchner wants to merge 2 commits into
RUrlus:stablefrom
JohannesBuchner:stable
Closed

let user retrieve the location of the dip#77
JohannesBuchner wants to merge 2 commits into
RUrlus:stablefrom
JohannesBuchner:stable

Conversation

@JohannesBuchner

Copy link
Copy Markdown
Contributor

A shot at #76

There are a few issues with the PR:

  • no tests
  • only very brief doc in the python and C++ function for the new parameter
  • data type is weird, it should be a single number -- it's been a while since I used C++; I copied the code from other objects, which use arrays, so this is a 1-element array.
  • potentially off by one bug, one in the --dipidx and one in the subtracting one at the end.

Would be cool to have this feature supported in diptest though!

@RUrlus

RUrlus commented Jul 10, 2025

Copy link
Copy Markdown
Owner

Ha @JohannesBuchner, thanks for taking a stab at it. I ran into some other things I wanted to fix at the same. I'm took your commit and went from there.
I don't have permission to push to your branch so I'll close this one and continue in #78

@RUrlus RUrlus closed this Jul 10, 2025
@JohannesBuchner

Copy link
Copy Markdown
Contributor Author

Great, thanks for picking this up

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants