search

dandi / dandi-schema

Schemata for DANDI archive project
Apache License 2.0
7 stars 10 forks source link

ENH: adopt sanitize_value from dandi-cli and use for sanitization of identifier #175

Closed yarikoptic closed 1 month ago

yarikoptic commented 1 year ago

Closes #172 as empirically shown

@satra do you see a quick way to add a test?

TODO

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.75%. Comparing base (1110194) to head (03748b9).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #175 +/- ## ========================================== + Coverage 97.74% 97.75% +0.01% ========================================== Files 16 16 Lines 1727 1739 +12 ========================================== + Hits 1688 1700 +12 Misses 39 39 ``` | [Flag](https://app.codecov.io/gh/dandi/dandi-schema/pull/175/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/dandi/dandi-schema/pull/175/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi) | `97.75% <100.00%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

satra commented 1 year ago

you could use an asset in the tests directory and change the "path" key and subject identifier keys to make the aggregate function raise that assertion you put.

yarikoptic commented 1 year ago

I didn't add assertion in this PR. In this PR such assertion would not be appropriate since code would still allow for multiple subjects from a single path.

satra commented 1 year ago

you could manually assert in the test.

yarikoptic commented 1 year ago

you could manually assert in the test.

well, all tests assert "manually", the question what and in which test.... well -- since confirmed empirically, just feel free to either merge as is, or extend with the test as you see fit.

satra commented 5 months ago

@yarikoptic - this needs to be updated and merged for #172