Allow user to specify mandatory (if not empty) fields in organize

[yarikoptic]

Closes: #69

• finish the code (CLI option, which would list available entities from potential_fields
• add tests to ensure that it has desired effect

[codecov]

Codecov Report

Base: 89.15% // Head: 89.16% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (be00d45) compared to base (8e67482).
Patch coverage: 96.29% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1171   +/-   ##
=======================================
  Coverage   89.15%   89.16%           
=======================================
  Files          76       76           
  Lines        9510     9525   +15     
=======================================
+ Hits         8479     8493   +14     
- Misses       1031     1032    +1     

Flag	Coverage Δ
unittests	89.16% <96.29%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
dandi/organize.py	83.54% <93.33%> (-0.01%)	⬇️
dandi/cli/cmd_organize.py	100.00% <100.00%> (ø)
dandi/consts.py	100.00% <100.00%> (ø)
dandi/tests/test_organize.py	95.42% <100.00%> (+0.13%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jwodder]

@yarikoptic To be clear, by "mandatory" you mean "must be included in organized filenames (rather than only being included if necessary for disambiguation)", correct? There's probably a better way to word that.

EDIT: Would --required-field be a better option name?

[yarikoptic]

@yarikoptic To be clear, by "mandatory" you mean "must be included in organized filenames (rather than only being included if necessary for disambiguation)", correct?

correct

EDIT: Would --required-field be a better option name?

it seems we never exposed in API/CLI the "mandatory"

❯ git grep mandatory
dandi/organize.py:    "subject_id": {"format": "sub-{}", "type": "mandatory"},
dandi/organize.py:    "modalities": {"format": "_{}", "type": "mandatory_if_not_empty"},
dandi/organize.py:    "extension": {"format": "{}", "type": "mandatory"},
dandi/organize.py:    "mandatory",
dandi/organize.py:    "mandatory_if_not_empty",
dandi/organize.py:            # we will add _mandatory_if_not_empty to those files records, which will
dandi/organize.py:                            r["_mandatory_if_not_empty"] = r.get(
dandi/organize.py:                                "_mandatory_if_not_empty", []
dandi/organize.py:    # unless it is mandatory, we would not include the fields with more than
dandi/organize.py:                (field_type == "mandatory")
dandi/organize.py:                    field_type == "mandatory_if_not_empty"
dandi/organize.py:                    or (field in r.get("_mandatory_if_not_empty", []))
dandi/validate.py:        if pattern.get("mandatory") or pattern.get("required"):
versioneer.py:    VCS = parser.get("versioneer", "VCS")  # mandatory

and I agree that required is a better word. I am ok to go with required instead of mandatory if all instances (besides versioneer) are also changed to required as well so we have it consistent at CLI and internal code levels.

[jwodder]

@yarikoptic How exactly should I test this? Are any of the simple*_nwb fixtures guaranteed to have certain fields that are nonempty yet not present by default in their organized filenames?

[yarikoptic]

@yarikoptic How exactly should I test this? Are any of the simple*_nwb fixtures guaranteed to have certain fields that are nonempty yet not present by default in their organized filenames?

if currently you organize a single file, it would not use any of the entities besides "mandatory" (sub). So you can have a test organizing a file requiring all other fields which are present (or not) in that file metadata -- and see if filename would reflect that.

[jwodder]

@yarikoptic Done?

[yarikoptic]

why was it moved? because otherwise not used anywhere?

[jwodder]

potential_fields now needs to be imported from organize.py in order to be used in the --required-field definition, but that leads to a bunch of unconditional "heavy" imports unless this line (and others) is moved from the module-level to function-level.

[yarikoptic]

Commit just says Fix heavy imports but what is "fixed" since it was there before -- is there a test fail or what?

Indeed for dandi * --help we might want to avoid heavy imports (but then should be done at the imports of cmd_ files) -- I would not bother trying to delay such a generic import as numpy. Not yet sure about other imports

[jwodder]

Yes, this test was failing.

[yarikoptic]

I see - thanks! So let's move potential_fields into consts.py as dandi_layout_fields variable, import from there where needed. .consts is a lightweight import. And revert this imports fixing commit.

[jwodder]

Done.

[yarikoptic]

Let's release whenever this one is done.

[yarikoptic]

Great! Thank you @jwodder !