Skip to content

Count only for the first hit for subject or tissue within filename#173

Merged
yarikoptic merged 2 commits intomasterfrom
bf-robustify-nsubjects
Mar 12, 2026
Merged

Count only for the first hit for subject or tissue within filename#173
yarikoptic merged 2 commits intomasterfrom
bf-robustify-nsubjects

Conversation

@yarikoptic
Copy link
Member

Also added assertion so we do not count incorrectly. But may be should be just a warning?

Closes #172

@yarikoptic yarikoptic requested a review from satra April 6, 2023 18:16
@satra
Copy link
Member

satra commented Apr 6, 2023

i think this also requires line 302 replaced with the sanitize function from dandi-cli.

@yarikoptic yarikoptic force-pushed the bf-robustify-nsubjects branch from fecfb26 to 7b9a460 Compare April 6, 2023 20:54
@codecov
Copy link

codecov bot commented Apr 6, 2023

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.92%. Comparing base (6b7332c) to head (b749796).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #173   +/-   ##
=======================================
  Coverage   97.91%   97.92%           
=======================================
  Files          18       18           
  Lines        2401     2405    +4     
=======================================
+ Hits         2351     2355    +4     
  Misses         50       50           
Flag Coverage Δ
unittests 97.92% <100.00%> (+<0.01%) ⬆️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yarikoptic
Copy link
Member Author

i think this also requires line 302 replaced with the sanitize function from dandi-cli.

did you try that and it resolved #172? would better be done in a separate PR then since this one does resolve issue too

@yarikoptic yarikoptic force-pushed the bf-robustify-nsubjects branch from 7b9a460 to ed02dec Compare April 6, 2023 21:24
@yarikoptic
Copy link
Member Author

yarikoptic commented Apr 6, 2023

i think this also requires line 302 replaced with the sanitize function from dandi-cli.

did you try that and it resolved #172? would better be done in a separate PR then since this one does resolve issue too

I tested, it does. Will submit a complimentary PR.

@yarikoptic
Copy link
Member Author

yarikoptic commented Apr 17, 2023

@satra so what do you think about this one? It is complimentary to #175

Also added assertion so we do not count incorrectly. But may be should
be just a warning?

Closes #172
@yarikoptic yarikoptic force-pushed the bf-robustify-nsubjects branch from d22ac9c to b749796 Compare March 12, 2026 19:32
@yarikoptic
Copy link
Member Author

ok, i will proceed with this (rebased -- still "worked").
@satra if you have more on this -- please chime in (ideally with a PR)

@yarikoptic yarikoptic added the patch Increment the patch version when merged label Mar 12, 2026
@yarikoptic yarikoptic merged commit 4b89e4f into master Mar 12, 2026
85 checks passed
@yarikoptic yarikoptic deleted the bf-robustify-nsubjects branch March 12, 2026 20:53
@satra
Copy link
Member

satra commented Mar 12, 2026

@yarikoptic - didn't see the ping. this will break number of samples count. it will always be 1. that part should continue to append new tissue samples.

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

Labels

patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect summaries (at least number of subjects) in some dandisets

2 participants