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

Merge RC_v1.5_hotfix into main #323

Merged
merged 23 commits into from
Sep 25, 2023
Merged

Merge RC_v1.5_hotfix into main #323

merged 23 commits into from
Sep 25, 2023

Conversation

w-k-jones
Copy link
Member

Hotfix v1.5.1 release for issue with strict thresholding found in #314 and #315, fixed by #316

  • Have you followed our guidelines in CONTRIBUTING.md?
  • Have you self-reviewed your code and corrected any misspellings?
  • Have you written documentation that is easy to understand?
  • Have you written descriptive commit messages?
  • Have you added NumPy docstrings for newly added functions?
  • Have you formatted your code using black?
  • If you have introduced a new functionality, have you added adequate unit tests?
  • Have all tests passed in your local clone?
  • If you have introduced a new functionality, have you added an example notebook?
  • Have you kept your pull request small and limited so that it is easy to review?
  • Have the newest changes from this branch been merged?

… and provides the same results as strict_thresholding=False if n_min_threshold is a fixed value
Reimplimentation of strict thresholding
@w-k-jones w-k-jones self-assigned this Aug 15, 2023
@w-k-jones w-k-jones added the release Organising the next release on conda-forge label Aug 15, 2023
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Patch coverage: 53.48% and project coverage change: -0.30% ⚠️

Comparison is base (d819159) 56.04% compared to head (513a074) 55.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
- Coverage   56.04%   55.75%   -0.30%     
==========================================
  Files          15       15              
  Lines        3290     3313      +23     
==========================================
+ Hits         1844     1847       +3     
- Misses       1446     1466      +20     
Flag Coverage Δ
unittests 55.75% <53.48%> (-0.30%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
tobac/utils/__init__.py 100.00% <ø> (ø)
tobac/utils/general.py 70.84% <12.50%> (-1.63%) ⬇️
tobac/feature_detection.py 83.94% <60.00%> (-2.53%) ⬇️
tobac/utils/internal.py 87.67% <75.00%> (-0.30%) ⬇️
tobac/__init__.py 92.30% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@w-k-jones w-k-jones mentioned this pull request Aug 15, 2023
8 tasks
@w-k-jones w-k-jones added this to the Version 1.5.x milestone Aug 15, 2023
@w-k-jones w-k-jones marked this pull request as draft August 15, 2023 15:33
@freemansw1
Copy link
Member

Assuming #322 is resolved, I'm happy for this release to be pushed.

@w-k-jones w-k-jones linked an issue Aug 15, 2023 that may be closed by this pull request
8 tasks
@w-k-jones w-k-jones removed a link to an issue Aug 15, 2023
8 tasks
@JuliaKukulies JuliaKukulies mentioned this pull request Aug 15, 2023
11 tasks
@w-k-jones w-k-jones marked this pull request as ready for review August 15, 2023 18:30
@w-k-jones
Copy link
Member Author

Assuming #322 is resolved, I'm happy for this release to be pushed.

We'll have to dig into the behaviour in #322 to see if anything is wrong or if that is expected behaviour. That may take a while, so perhaps if there is a fix needed there we should delay for a future release?

@freemansw1
Copy link
Member

Assuming #322 is resolved, I'm happy for this release to be pushed.

We'll have to dig into the behaviour in #322 to see if anything is wrong or if that is expected behaviour. That may take a while, so perhaps if there is a fix needed there we should delay for a future release?

I'll defer to your judgement here and am happy to merge and fix that later.

Copy link
Member

@JuliaKukulies JuliaKukulies left a comment

Choose a reason for hiding this comment

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

Good to go! I agree that we can wait with #322 because it is also not clear for me if this is actually a bug.

Copy link
Member

@freemansw1 freemansw1 left a comment

Choose a reason for hiding this comment

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

I am happy to merge/release!

@freemansw1 freemansw1 self-requested a review September 14, 2023 16:07
Copy link
Member

@freemansw1 freemansw1 left a comment

Choose a reason for hiding this comment

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

Now that #335 is in, I think this is ready for merge/release.

Recategorise and reorder v1.5.1 changelogs to make order of changes clearer
@freemansw1
Copy link
Member

Ah, good catch on the changelog.

@w-k-jones w-k-jones merged commit 57385cd into main Sep 25, 2023
11 of 12 checks passed
@w-k-jones w-k-jones deleted the RC_v1.5_hotfix branch September 25, 2023 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Organising the next release on conda-forge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants