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

Feature/window splitting try2 #21

Merged

Conversation

jspaezp
Copy link
Contributor

@jspaezp jspaezp commented Jul 17, 2024

Since there were so many changes this is pretty much a "as a reference" PR ... we need to cherry pick what changes are actually compatible and move them forward onto the new base.

@sander-willems-bruker
Copy link
Collaborator

As of this point I'll not push to develop straight away anymore and maintain my own develop branch for easy of collaboration. Apologies if I got your PR in a "messy" state due to the merge conflicts. Most of the conflicts should be easily resolved though, as most of them should be the result of error propagation more than anything else. If I didn't crush your motivation, feel free to make the PR with merge conflicts, I will handle resolve them locally before final merging into dev.

@jspaezp
Copy link
Contributor Author

jspaezp commented Jul 17, 2024

Checklist (I take ideas on how to/how should be handled):

  1. Remove debug prints
  2. Add unit tests (should be easy to re-use the current files...)
  3. Handle edges (Right now for instance if the scan window is 150 scans and the step-span is 100-100, it will only yield a single scan from 0-100, and thus drop the last 50)

@jspaezp
Copy link
Contributor Author

jspaezp commented Jul 25, 2024

I am not 100% happy with the extent of the tests but i think it can be "good enough" for some purposes.... at least good enough to review.

@jspaezp jspaezp marked this pull request as ready for review July 25, 2024 16:17
@sander-willems-bruker
Copy link
Collaborator

Merged it into my branch which is ready to go into develop: #23

@sander-willems-bruker sander-willems-bruker merged commit 5f7a3de into MannLabs:develop Jul 31, 2024
1 check passed
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