Skip to content

Commit eeca6cf

Browse files
aaron-skydioaaron-skydio
committed
[SymForce] Make wheels pass tests
Changes to enable running python tests on wheels. Not running C++ tests, since 1) we don't ship SymForce headers in wheels and 2) building the tests against the wheel would take some work The following PR enables these tests in the build-wheels CI job. Topic: sf-wheel-tests Relative: sf-fix-macos-wheels GitOrigin-RevId: 4fe2a191482991c939ddabff002e5999f4eb8234
1 parent c318124 commit eeca6cf

30 files changed

+194
-164
lines changed

‎pyproject.toml‎

Lines changed: 0 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -49,138 +49,6 @@ Source = "https://github.com/symforce-org/symforce"
4949
[tool.setuptools_scm]
5050
# Empty, presence enables setuptools_scm
5151

52-
# --------------------------------------------------------------------------------
53-
# Ruff
54-
# --------------------------------------------------------------------------------
55-
56-
[tool.ruff]
57-
line-length = 100
58-
exclude = ["third_party", "build", ".eggs", "lcmtypes", "*.pyi"]
59-
extend-include = ["*.ipynb"]
60-
61-
[tool.ruff.format]
62-
line-ending = "lf"
63-
64-
[tool.ruff.lint]
65-
preview = true
66-
typing-modules = ["symforce.typing"]
67-
68-
select = [
69-
"A", # flake8
70-
"B", # flake8-bugbear
71-
"D", # docstrings
72-
"E4", # pycodestyle - ERROR - Imports - E401, E402
73-
"E7", # pycodestyle - ERROR - Statements - E701-E743
74-
"E9", # pycodestyle - ERROR - Runtime - E901, E902
75-
"F", # pyflakes - ALL - F401-F901
76-
"I", # isort
77-
"PLC", # pylint - convention
78-
"PLE", # pylint - error
79-
"PLR", # pylint - refactor
80-
"PLW", # pylint - warning
81-
"RUF100", # unused-noqa
82-
"SLF", # flake8-self
83-
"TC", # flake8-type-checking
84-
"TD", # flake8-todos
85-
]
86-
87-
ignore = [
88-
# --------------------------------------------------------------------------------
89-
# Leave ignored
90-
# --------------------------------------------------------------------------------
91-
92-
# B (flake8-bugbear)
93-
"B008", # function-call-in-default-argument
94-
95-
# D (pydocstyle)
96-
# differences on top of default google conventions:
97-
"D1", # undocumented-*
98-
"D417", # undocumented-param
99-
"D200", # fits-on-one-line # Good rule, fix not always available
100-
"D202", # no-blank-line-after-function
101-
"D205", # blank-line-after-summary # Requires 1-line summaries, fix not always available
102-
"D212", # nulti-line-summary-first-line # Disagreement between Google and Skydio style
103-
"D402", # no-signature # Good rule, has false positives
104-
"D403", # first-line-capitalized
105-
"D415", # ends-in-punctuation
106-
107-
# E (pycodestyle)
108-
"E402", # Module level import not at top of file
109-
"E731", # Do not assign a `lambda` expression, use a `def`
110-
"E741", # Ambiguous variable name: `x`
111-
112-
# PL (pylint)
113-
"PLC0415", # import-outside-top-level
114-
"PLC2701", # Private name import `_x` from external module `y`
115-
"PLR2004", # magic-value-comparison
116-
"PLW1514", # unspecified-encoding
117-
"PLW1641", # eq-without-hash
118-
"PLW2901", # redefined-loop-name
119-
120-
# TC (flake8-type-checking)
121-
# SymForce does runtime resolution of annotations, so we disable rules that are designed with no
122-
# runtime resolution of annotations in mind.
123-
"TC001", # typing-only-first-party-import
124-
"TC002", # typing-only-third-party-import
125-
"TC003", # typing-only-standard-library-import
126-
"TC006", # runtime-cast-value
127-
"TC007", # unquoted-type-alias
128-
129-
# TD (flake8-todos)
130-
"TD003", # missing-todo-link
131-
132-
# --------------------------------------------------------------------------------
133-
# Maybe enable later
134-
# --------------------------------------------------------------------------------
135-
136-
"ARG", # flake8-unused-arguments
137-
"B011", # Do not `assert False`, raise `AssertionError()`
138-
]
139-
140-
extend-select = [
141-
"D213", # multi-line-summary-second-line # Disagreement between Google and SymForce style
142-
]
143-
144-
[tool.ruff.lint.flake8-builtins]
145-
builtins-ignorelist = ["__doc__"]
146-
147-
148-
[tool.ruff.lint.per-file-ignores]
149-
150-
# Unused imports in __init__.py
151-
"__init__.py" = ["F401"]
152-
153-
# Unbound loop variables in benchmark notebooks
154-
"symforce/benchmarks/notebooks/*.ipynb" = ["B023"]
155-
156-
# Imports shadowing builtins (like display)
157-
"**/*.ipynb" = ["A004"]
158-
159-
[tool.ruff.lint.isort]
160-
known-first-party = ["sym", "symforce"]
161-
force-single-line = true
162-
section-order = [
163-
"future",
164-
"standard-library",
165-
"third-party",
166-
"generated",
167-
"first-party",
168-
"local-folder",
169-
]
170-
171-
[tool.ruff.lint.isort.sections]
172-
"generated" = ["lcmtypes"]
173-
174-
[tool.ruff.lint.pylint]
175-
max-args = 10
176-
max-branches = 20
177-
max-locals = 20
178-
max-public-methods = 100
179-
max-returns = 10
180-
181-
[tool.ruff.lint.pydocstyle]
182-
convention = "google"
183-
18452
# --------------------------------------------------------------------------------
18553
# Mypy
18654
# --------------------------------------------------------------------------------

‎ruff.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
symforce/ruff.toml

‎setup.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,9 @@ def fixed_readme() -> str:
361361
"symengine": "third_party/symenginepy/symengine",
362362
"lcmtypes": "lcmtypes_build/lcmtypes",
363363
},
364-
package_data={"": ["*.jinja", "*.mtx", "README*", ".clang-format", "py.typed"]},
364+
package_data={
365+
"": ["*.jinja", "*.mtx", "README*", ".clang-format", "py.typed", "ruff.toml"]
366+
},
365367
# pyproject.toml doesn't allow specifying url or homepage separately, and if it's not
366368
# specified separately PyPI sorts all the links alphabetically
367369
# https://github.com/pypi/warehouse/issues/3097

‎symforce/benchmarks/matrix_multiplication/generate_matrix_multiplication_benchmark.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ def get_matrices() -> T.List[T.Tuple[str, Path, scipy.sparse.csr_matrix]]:
3838
matrices = []
3939

4040
for path in (Path(__file__).parent / "matrices").iterdir():
41-
if not path.is_dir():
41+
if not path.is_dir() or path.name == "__pycache__":
4242
continue
4343

4444
matrix_name = path.name

‎symforce/path_util.py‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
# ----------------------------------------------------------------------------
55

66
import json
7+
import os
78
import typing as T
89
from pathlib import Path
910

@@ -66,15 +67,24 @@ def symforce_root() -> Path:
6667
return Path(__file__).parent.parent
6768

6869

69-
def symforce_data_root() -> Path:
70+
def symforce_data_root(test_file_path: T.Optional[str]) -> Path:
7071
"""
7172
The root directory of the symforce project, for use accessing data that might need to be updated
72-
(such as generated files). Most of the time this is the same as :func:`symforce_root`, but when
73-
the ``--update`` flag is passed to a test, this is guaranteed to point to the *resolved*
74-
version, i.e. the actual symforce location on disk regardless of whether this path is a symlink.
73+
(such as generated files). Most of the time this is the same as :func:`symforce_root`, except
74+
for two cases:
75+
76+
1) When running tests on wheels, this is the root of the copy of SymForce containing the test,
77+
not the installed copy. The generated files we're checking aren't in the wheel, they're in
78+
the source tree.
79+
2) When the ``--update`` flag is passed to a test, this is guaranteed to point to the *resolved*
80+
version, i.e. the actual symforce location on disk regardless of whether this path is a
81+
symlink.
7582
"""
7683
from symforce.test_util.test_case_mixin import SymforceTestCaseMixin
7784

85+
if test_file_path is not None and "SYMFORCE_WHEEL_TESTS" in os.environ:
86+
return Path(test_file_path).resolve().parent.parent
87+
7888
if SymforceTestCaseMixin.should_update():
7989
return Path(__file__).resolve().parent.parent
8090
else:

‎symforce/ruff.toml‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
line-length = 100
2+
exclude = ["third_party", "build", ".eggs", "lcmtypes", "*.pyi"]
3+
extend-include = ["*.ipynb"]
4+
5+
[format]
6+
line-ending = "lf"
7+
8+
[lint]
9+
preview = true
10+
typing-modules = ["symforce.typing"]
11+
12+
select = [
13+
"A", # flake8
14+
"B", # flake8-bugbear
15+
"D", # docstrings
16+
"E4", # pycodestyle - ERROR - Imports - E401, E402
17+
"E7", # pycodestyle - ERROR - Statements - E701-E743
18+
"E9", # pycodestyle - ERROR - Runtime - E901, E902
19+
"F", # pyflakes - ALL - F401-F901
20+
"I", # isort
21+
"PLC", # pylint - convention
22+
"PLE", # pylint - error
23+
"PLR", # pylint - refactor
24+
"PLW", # pylint - warning
25+
"RUF100", # unused-noqa
26+
"SLF", # flake8-self
27+
"TC", # flake8-type-checking
28+
"TD", # flake8-todos
29+
]
30+
31+
ignore = [
32+
# --------------------------------------------------------------------------------
33+
# Leave ignored
34+
# --------------------------------------------------------------------------------
35+
36+
# B (flake8-bugbear)
37+
"B008", # function-call-in-default-argument
38+
39+
# D (pydocstyle)
40+
# differences on top of default google conventions:
41+
"D1", # undocumented-*
42+
"D417", # undocumented-param
43+
"D200", # fits-on-one-line # Good rule, fix not always available
44+
"D202", # no-blank-line-after-function
45+
"D205", # blank-line-after-summary # Requires 1-line summaries, fix not always available
46+
"D212", # nulti-line-summary-first-line # Disagreement between Google and Skydio style
47+
"D402", # no-signature # Good rule, has false positives
48+
"D403", # first-line-capitalized
49+
"D415", # ends-in-punctuation
50+
51+
# E (pycodestyle)
52+
"E402", # Module level import not at top of file
53+
"E731", # Do not assign a `lambda` expression, use a `def`
54+
"E741", # Ambiguous variable name: `x`
55+
56+
# PL (pylint)
57+
"PLC0415", # import-outside-top-level
58+
"PLC2701", # Private name import `_x` from external module `y`
59+
"PLR2004", # magic-value-comparison
60+
"PLW1514", # unspecified-encoding
61+
"PLW1641", # eq-without-hash
62+
"PLW2901", # redefined-loop-name
63+
64+
# TC (flake8-type-checking)
65+
# SymForce does runtime resolution of annotations, so we disable rules that are designed with no
66+
# runtime resolution of annotations in mind.
67+
"TC001", # typing-only-first-party-import
68+
"TC002", # typing-only-third-party-import
69+
"TC003", # typing-only-standard-library-import
70+
"TC006", # runtime-cast-value
71+
"TC007", # unquoted-type-alias
72+
73+
# TD (flake8-todos)
74+
"TD003", # missing-todo-link
75+
76+
# --------------------------------------------------------------------------------
77+
# Maybe enable later
78+
# --------------------------------------------------------------------------------
79+
80+
"ARG", # flake8-unused-arguments
81+
"B011", # Do not `assert False`, raise `AssertionError()`
82+
]
83+
84+
extend-select = [
85+
"D213", # multi-line-summary-second-line # Disagreement between Google and SymForce style
86+
]
87+
88+
[lint.flake8-builtins]
89+
builtins-ignorelist = ["__doc__"]
90+
91+
92+
[lint.per-file-ignores]
93+
94+
# Unused imports in __init__.py
95+
"__init__.py" = ["F401"]
96+
97+
# Unbound loop variables in benchmark notebooks
98+
"benchmarks/notebooks/*.ipynb" = ["B023"]
99+
100+
# Imports shadowing builtins (like display)
101+
"**/*.ipynb" = ["A004"]
102+
103+
[lint.isort]
104+
known-first-party = ["sym", "symforce"]
105+
force-single-line = true
106+
section-order = [
107+
"future",
108+
"standard-library",
109+
"third-party",
110+
"generated",
111+
"first-party",
112+
"local-folder",
113+
]
114+
115+
[lint.isort.sections]
116+
"generated" = ["lcmtypes"]
117+
118+
[lint.pylint]
119+
max-args = 10
120+
max-branches = 20
121+
max-locals = 20
122+
max-public-methods = 100
123+
max-returns = 10
124+
125+
[lint.pydocstyle]
126+
convention = "google"

‎symforce/test_util/__init__.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
from .test_case import TestCase
1111
from .test_case import expected_failure_on_sympy
12+
from .test_case import requires_source_build
1213
from .test_case import slow_on_sympy
1314
from .test_case import symengine_only
1415
from .test_case import sympy_only

‎symforce/test_util/test_case.py‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# This source code is under the Apache 2.0 license found in the LICENSE file.
44
# ----------------------------------------------------------------------------
55

6+
import os
67
import random
78
import sys
89
import unittest
@@ -61,6 +62,16 @@ def setUp(self) -> None:
6162
self.verbose = ("-v" in sys.argv) or ("--verbose" in sys.argv)
6263

6364

65+
def requires_source_build(func: T.Callable) -> T.Callable:
66+
"""
67+
Decorator to mark a test as skipped for wheel installs (as opposed to from-source builds)
68+
"""
69+
if "SYMFORCE_WHEEL_TESTS" in os.environ:
70+
return unittest.skip("Skipping for wheel install")(func)
71+
else:
72+
return func
73+
74+
6475
def sympy_only(func: T.Callable) -> T.Callable:
6576
"""
6677
Decorator to mark a test to only run on SymPy, and skip otherwise.
@@ -83,7 +94,7 @@ def symengine_only(func: T.Callable) -> T.Callable:
8394

8495
def expected_failure_on_sympy(func: T.Callable) -> T.Callable:
8596
"""
86-
Decorator to mark a test to be expected to fail only on SymPy..
97+
Decorator to mark a test to be expected to fail only on SymPy
8798
"""
8899
if symforce.get_symbolic_api() == "sympy":
89100
return unittest.expectedFailure(func)

‎test/symforce_benchmarks_codegen_test.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from symforce.test_util import TestCase
1414
from symforce.test_util import sympy_only
1515

16-
BENCHMARKS_DIR = path_util.symforce_data_root() / "symforce" / "benchmarks"
16+
BENCHMARKS_DIR = path_util.symforce_data_root(__file__) / "symforce" / "benchmarks"
1717

1818

1919
class SymforceBenchmarksCodegenTest(TestCase):

‎test/symforce_cc_sym_stubs_codegen_test.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ def test_generate_cc_sym_stubs(self) -> None:
111111

112112
self.compare_or_update_file(
113113
new_file=output_dir / "cc_sym.pyi",
114-
path=path_util.symforce_data_root() / "symforce" / "pybind" / "cc_sym.pyi",
114+
path=path_util.symforce_data_root(__file__) / "symforce" / "pybind" / "cc_sym.pyi",
115115
)
116116

117117

0 commit comments

Comments
 (0)