Remove ValidationResult.path_regex as possible return of purview()

[candleindark]

A purview is expected to be the string expression of a path or None in its only usages in _process_issues() as indicated in the comments shown below, so it is inappropriate to return ValidationResult.path_regex which is a regex.

dandi-cli/dandi/cli/cmd_validate.py

Lines 157 to 158 in e734c68

	# The purviews are the paths, if we group by path, we need to de-duplicate.
	# typing complains if we just take the set, though the code works otherwise.

Note: This change will pave the way to implement a solution for #1597.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.08%. Comparing base (e734c68) to head (9367939).
⚠️ Report is 133 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1738   +/-   ##
=======================================
  Coverage   75.08%   75.08%           
=======================================
  Files          84       84           
  Lines       11874    11872    -2     
=======================================
- Hits         8915     8914    -1     
+ Misses       2959     2958    -1     

Flag	Coverage Δ
unittests	75.08% <ø> (+<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.

[Copilot]

Pull Request Overview

This PR removes the conditional check for path_regex in the purview property of the ValidationResult class. The change simplifies the property's logic by reducing the fallback chain from four conditions (path → path_regex → dataset_path → None) to three (path → dataset_path → None).

Key Changes

• Simplified the purview property logic by removing the path_regex fallback condition

Comments suppressed due to low confidence (1)

dandi/validate_types.py:225

• Removing the path_regex check breaks the purview property for ValidationResult objects that have path_regex set but no path. This affects BIDS validation results (see dandi/validate.py line 107) and DANDI layout validation results (see dandi/organize.py lines 1150, 1166). When these validation results are displayed via dandi/cli/cmd_validate.py, they will now incorrectly return dataset_path or None instead of the expected path_regex value. The removed lines should be restored:\npython\nelif self.path_regex is not None:\n return self.path_regex\n

        elif self.dataset_path is not None:
            return str(self.dataset_path)
        else:
            return None

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yarikoptic]

well, it could be the actual string representation of the regex, and that is what I guess it was expected to be. That would have allowed to point to group of files right away. I guess the real underlying issue is that with bids-validator-deno we do not have regexes and thus it would not be applicable. But is it really required for #1597 ?

[candleindark]

well, it could be the actual string representation of the regex, and that is what I guess it was expected to be. That would have allowed to point to group of files right away.

You are right. The purview method is really just a part of an organization scheme for displaying the errors. With that said, the following is some considerations.

In the current definition of purview, the definition before this proposed change, the other two possible non-none values are ValidaitonResult.path and ValidationResult.dataset_path. Both values are the absolute path of a file/folder that exists in the filesystem according to usage instances of ValidationResult I have analyzed. On the other hand, ValidationResult.path_regex is a regex used in validating the last component of a path as in

dandi-cli/dandi/organize.py

Lines 1141 to 1169 in e734c68

	if not re.fullmatch(ORGANIZED_FILENAME_REGEX, path.name):
	errors.append(
	ValidationResult(
	id="DANDI.NON_DANDI_FILENAME",
	origin=ORIGIN_VALIDATION_DANDI_LAYOUT,
	severity=Severity.ERROR,
	scope=Scope.FILE,
	path=filepath,
	message="Filename does not conform to DANDI standard",
	path_regex=ORGANIZED_FILENAME_REGEX,
	dandiset_path=dandiset_path,
	)
	)
	if not (
	len(path.parent.parts) == 1
	and re.fullmatch(ORGANIZED_FOLDER_REGEX, str(path.parent))
	):
	errors.append(
	ValidationResult(
	id="DANDI.NON_DANDI_FOLDERNAME",
	origin=ORIGIN_VALIDATION_DANDI_LAYOUT,
	severity=Severity.ERROR,
	scope=Scope.FOLDER,
	path=filepath,
	message="File is not in folder at root with subject name",
	path_regex=ORGANIZED_FOLDER_REGEX,
	dandiset_path=dandiset_path,
	)
	)

Particularly, it is a regex used in validating the last component of a path. It is not necessarily the case that the component matches the regex. It can be the case that the component doesn't match the regex as in

dandi-cli/dandi/organize.py

Lines 1141 to 1169 in e734c68

	if not re.fullmatch(ORGANIZED_FILENAME_REGEX, path.name):
	errors.append(
	ValidationResult(
	id="DANDI.NON_DANDI_FILENAME",
	origin=ORIGIN_VALIDATION_DANDI_LAYOUT,
	severity=Severity.ERROR,
	scope=Scope.FILE,
	path=filepath,
	message="Filename does not conform to DANDI standard",
	path_regex=ORGANIZED_FILENAME_REGEX,
	dandiset_path=dandiset_path,
	)
	)
	if not (
	len(path.parent.parts) == 1
	and re.fullmatch(ORGANIZED_FOLDER_REGEX, str(path.parent))
	):
	errors.append(
	ValidationResult(
	id="DANDI.NON_DANDI_FOLDERNAME",
	origin=ORIGIN_VALIDATION_DANDI_LAYOUT,
	severity=Severity.ERROR,
	scope=Scope.FOLDER,
	path=filepath,
	message="File is not in folder at root with subject name",
	path_regex=ORGANIZED_FOLDER_REGEX,
	dandiset_path=dandiset_path,
	)
	)

I guess the real underlying issue is that with bids-validator-deno we do not have regexes and thus it would not be applicable. But is it really required for #1597 ?

No, you are right about this. The purview method is really just a part of an organization scheme for displaying the errors.

Let's close this PR if you are OK with what purview currently is after reading the analysis I provided.

[yarikoptic]

as no problem this would solve was demonstrated, I would just keep it as is for now.