Skip to content

Fix DTD partition validation using and instead of or - #9561

Open
Osamaali313 wants to merge 1 commit into
pytorch:mainfrom
Osamaali313:fix/dtd-partition-validation
Open

Fix DTD partition validation using and instead of or#9561
Osamaali313 wants to merge 1 commit into
pytorch:mainfrom
Osamaali313:fix/dtd-partition-validation

Conversation

@Osamaali313

Copy link
Copy Markdown

Problem

DTD.__init__ (torchvision/datasets/dtd.py) validates the partition argument with and:

if not isinstance(partition, int) and not (1 <= partition <= 10):
    raise ValueError(
        f"Parameter 'partition' should be an integer with `1 <= partition <= 10`, "
        f"but got {partition} instead"
    )

The intent is to reject any value that isn't an int in [1, 10]. The correct predicate is not (isinstance(partition, int) and 1 <= partition <= 10), which by De Morgan is not isinstance(...) or not (1 <= ... <= 10). With and, the error fires only when the value is both a non-int and out of range — so a normal out-of-range int like 11 (not isinstance(11, int)False) short-circuits the and to False and passes validation.

Impact

DTD(root, partition=11) (or 0, -3) slips past the check and then dies later at:

open(self._meta_folder / f"{self._split}{self._partition}.txt")

with an opaque FileNotFoundError: ...train11.txt instead of the documented ValueError. A str partition like "1" raises TypeError from 1 <= "1" for the same reason (the or fix short-circuits before the comparison and handles that too). torchvision.datasets.DTD is public and partition is a documented constructor arg; the error message in this very statement documents the contract the guard fails to enforce.

Reproduction

partition before (and) after (or)
1, 10 accepted accepted
11, 0, -3 accepted ❌ (later FileNotFoundError) ValueError
"1" TypeError ValueError

Fix

if not isinstance(partition, int) or not (1 <= partition <= 10):

`DTD.__init__` validates the `partition` argument with:

    if not isinstance(partition, int) and not (1 <= partition <= 10):
        raise ValueError(...)

The intent is to reject any value that is not an int in [1, 10]. The correct
predicate is `not (isinstance(partition, int) and 1 <= partition <= 10)`,
which by De Morgan is `not isinstance(...) or not (1 <= ... <= 10)`. Using
`and` means the error is raised only when the value is simultaneously a
non-int and out of range, so a normal out-of-range int like `11` (where
`not isinstance(11, int)` is False) short-circuits and slips through.

`DTD(root, partition=11)` then fails later at
`open(... f"{self._split}{self._partition}.txt")` with an opaque
`FileNotFoundError: ...train11.txt` instead of the documented ValueError.
A str partition like "1" raises `TypeError` from `1 <= "1"` for the same
reason; the `or` fix short-circuits before the comparison and handles that
too. The error message in this same statement documents the intended
contract ("integer with 1 <= partition <= 10"). Use `or`.
Copilot AI review requested due to automatic review settings July 19, 2026 21:09
@pytorch-bot

pytorch-bot Bot commented Jul 19, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/9561

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the cla signed label Jul 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants