Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude test data from the Python package #5970

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions onnx/backend/test/case/node/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
# SPDX-License-Identifier: Apache-2.0
from __future__ import annotations

import pathlib
import subprocess
import sys
from copy import deepcopy
from pathlib import Path
from typing import Any, Callable, Sequence

import numpy as np
Expand Down Expand Up @@ -389,7 +389,7 @@


def get_diff_op_types():
cwd_path = Path.cwd()
cwd_path = pathlib.Path.cwd()

Check warning on line 392 in onnx/backend/test/case/node/__init__.py

View check run for this annotation

Codecov / codecov/patch

onnx/backend/test/case/node/__init__.py#L392

Added line #L392 was not covered by tests
# git fetch first for git diff on GitHub Action
subprocess.run(
["git", "fetch", "origin", "main:main"],
Expand Down
6 changes: 1 addition & 5 deletions onnx/backend/test/loader/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,9 @@

from onnx.backend.test.case.test_case import TestCase

DATA_DIR = os.path.join(
os.path.dirname(os.path.realpath(os.path.dirname(__file__))), "data"
)


def load_model_tests(
data_dir: str = DATA_DIR,
data_dir: str | os.PathLike,
kind: str | None = None,
) -> list[TestCase]:
"""Load model test cases from on-disk data files."""
Expand Down
28 changes: 13 additions & 15 deletions onnx/backend/test/runner/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import functools
import glob
import os
import pathlib
import re
import shutil
import sys
Expand Down Expand Up @@ -57,6 +58,7 @@ class Runner:
def __init__(
self,
backend: type[Backend],
test_data_dir: str | os.PathLike,
parent_module: str | None = None,
test_kwargs: dict | None = None,
) -> None:
Expand All @@ -66,26 +68,27 @@ def __init__(
self._exclude_patterns: set[Pattern[str]] = set()
self._xfail_patterns: set[Pattern[str]] = set()
self._test_kwargs: dict = test_kwargs or {}
self._test_data_dir = test_data_dir

# This is the source of the truth of all test functions.
# Properties `test_cases`, `test_suite` and `tests` will be
# derived from it.
# {category: {name: func}}
self._test_items: dict[str, dict[str, TestItem]] = defaultdict(dict)

for rt in load_model_tests(kind="node"):
for rt in load_model_tests(data_dir=test_data_dir, kind="node"):
self._add_model_test(rt, "Node")

for rt in load_model_tests(kind="real"):
for rt in load_model_tests(data_dir=test_data_dir, kind="real"):
self._add_model_test(rt, "Real")

for rt in load_model_tests(kind="simple"):
for rt in load_model_tests(data_dir=test_data_dir, kind="simple"):
self._add_model_test(rt, "Simple")

for ct in load_model_tests(kind="pytorch-converted"):
for ct in load_model_tests(data_dir=test_data_dir, kind="pytorch-converted"):
self._add_model_test(ct, "PyTorchConverted")

for ot in load_model_tests(kind="pytorch-operator"):
for ot in load_model_tests(data_dir=test_data_dir, kind="pytorch-operator"):
self._add_model_test(ot, "PyTorchOperator")

def _get_test_case(self, name: str) -> type[unittest.TestCase]:
Expand Down Expand Up @@ -345,17 +348,12 @@ def run(test_self: Any, device: str, **kwargs) -> None: # noqa: ARG001
if model_test.url is not None and model_test.url.startswith(
"onnx/backend/test/data/light/"
):
# Get the root directory from the test data dir in a
# hacky way. This will break when moving the files
# elsewhere.
root = pathlib.Path(self._test_data_dir).parent.parent.parent.parent
# testing local files
model_pb_path = os.path.normpath(
os.path.join(
os.path.dirname(__file__),
"..",
"..",
"..",
"..",
model_test.url,
)
)
model_pb_path = root / model_test.url
if not os.path.exists(model_pb_path):
raise FileNotFoundError(f"Unable to find model {model_pb_path!r}.")
onnx_home = os.path.expanduser(
Expand Down
15 changes: 7 additions & 8 deletions onnx/backend/test/stat_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from __future__ import annotations

import os
import pathlib

Check warning on line 9 in onnx/backend/test/stat_coverage.py

View check run for this annotation

Codecov / codecov/patch

onnx/backend/test/stat_coverage.py#L9

Added line #L9 was not covered by tests
from typing import IO, Any, Sequence

from onnx import AttributeProto, defs, load
Expand Down Expand Up @@ -99,13 +100,8 @@
f.write("Node tests have covered 0/0 (N/A) common operators. \n\n")
if num_experimental:
f.write(
"Node tests have covered {}/{} ({:.2f}%, {} generators excluded) " # noqa: UP032
"experimental operators.\n\n".format(
len(experimental_covered),
num_experimental,
(len(experimental_covered) / float(num_experimental) * 100),
len(experimental_generator),
)
f"Node tests have covered {len(experimental_covered)}/{num_experimental} ({len(experimental_covered) / float(num_experimental) * 100:.2f}%, {len(experimental_generator)} generators excluded) "
"experimental operators.\n\n"
)
else:
f.write("Node tests have covered 0/0 (N/A) experimental operators.\n\n")
Expand Down Expand Up @@ -158,7 +154,10 @@
# Need to grab associated nodes
attrs: dict[str, dict[str, list[Any]]] = {}
model_paths: list[Any] = []
for rt in load_model_tests(kind="real"):

for rt in load_model_tests(
data_dir=pathlib.Path(__file__).parent / "data", kind="real"
):
if rt.url.startswith("onnx/backend/test/data/light/"):
# testing local files
model_name = os.path.normpath(
Expand Down
7 changes: 6 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,21 @@ onnx = [
"**/*.h",
"**/*.proto",
"**/*.pyi",
"backend/test/data/**/*",
"py.typed",
]

[tool.setuptools.exclude-package-data]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the change goes into the right direction. My only concern is runtimes such as onnxruntime expects to find this data. I'd me more confortable to display an error message saying what instruction is needed to get the data and have the backend tests passing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

onnxruntime already fetches the entire onnx source during the build process anyway. Are they really using the PyPI package for testing, too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it is used. onnx should be shipped without data. That's not a debate. Since this data can be automatically generated, we could also generate this data when the package is being installed. Something like pip install onnx <options>... So developers using the data could just install onnx with this option and nothing else would change.

onnx = [
"backend/test/data/**/*",
]

[tool.pytest.ini_options]

addopts = "--nbval --nbval-current-env --tb=short --color=yes"
testpaths = [
"onnx/test",
"onnx/examples",
"tests",
]

[tool.mypy]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from __future__ import annotations

import pathlib
import platform
import unittest
from typing import Any
Expand All @@ -26,6 +27,7 @@
ort: Any = None # type: ignore[no-redef]
ort_version: Any = None # type: ignore[no-redef]

TEST_DATA_DIR = pathlib.Path(__file__).parent.parent.parent / "onnx/backend/test/data"

# The following just executes a backend based on InferenceSession through the backend test

Expand Down Expand Up @@ -108,7 +110,9 @@ def run_node(cls, node, inputs, device=None, outputs_info=None, **kwargs):


if ort is not None:
backend_test = onnx.backend.test.BackendTest(InferenceSessionBackend, __name__)
backend_test = onnx.backend.test.BackendTest(
InferenceSessionBackend, test_data_dir=TEST_DATA_DIR, parent_module=__name__
)

if platform.architecture()[0] == "32bit":
backend_test.exclude("(test_vgg19|test_zfnet|test_bvlc_alexnet)")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
from __future__ import annotations

import os
import pathlib
import platform
import sys
import unittest
from typing import Any

import numpy
import version_utils
import packaging

import onnx.backend.base
import onnx.backend.test
Expand All @@ -20,6 +21,8 @@
from onnx.backend.base import Device, DeviceType
from onnx.reference import ReferenceEvaluator

_TEST_DATA_DIR = pathlib.Path(__file__).parent.parent.parent / "onnx/backend/test/data"

# The following just executes a backend based on ReferenceEvaluator through the backend test


Expand Down Expand Up @@ -90,7 +93,9 @@ def run_node(cls, node, inputs, device=None, outputs_info=None, **kwargs):
raise NotImplementedError("Unable to run the model node by node.")


backend_test = onnx.backend.test.BackendTest(ReferenceEvaluatorBackend, __name__)
backend_test = onnx.backend.test.BackendTest(
ReferenceEvaluatorBackend, test_data_dir=_TEST_DATA_DIR, parent_module=__name__
)

if os.getenv("APPVEYOR"):
backend_test.exclude("(test_vgg19|test_zfnet)")
Expand Down Expand Up @@ -194,7 +199,7 @@ def run_node(cls, node, inputs, device=None, outputs_info=None, **kwargs):
backend_test.exclude("test_qlinearmatmul_3D_int8_float32_cpu")

# op_dft and op_stft requires numpy >= 1.21.5
if version_utils.numpy_older_than("1.21.5"):
if packaging.version.parse(numpy.__version__) < packaging.version.parse("1.21.5"):
backend_test.exclude("test_stft")
backend_test.exclude("test_stft_with_window")
backend_test.exclude("test_stft_cpu")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import itertools
import os
import pathlib
import platform
import unittest
from typing import Any, Sequence
Expand All @@ -30,6 +31,8 @@
# We don't enable report in this test because the report collection logic itself
# fails when models are mal-formed.

_TEST_DATA_DIR = pathlib.Path(__file__).parent.parent.parent / "onnx/backend/test/data"


class DummyBackend(onnx.backend.base.Backend):
@classmethod
Expand Down Expand Up @@ -112,7 +115,10 @@ def do_enforce_test_coverage_safelist(model: ModelProto) -> bool:
}

backend_test = onnx.backend.test.BackendTest(
DummyBackend, __name__, test_kwargs=test_kwargs
DummyBackend,
test_data_dir=_TEST_DATA_DIR,
parent_module=__name__,
test_kwargs=test_kwargs,
)
if os.getenv("APPVEYOR"):
backend_test.exclude(r"(test_vgg19|test_zfnet)")
Expand Down