Skip to content

Commit f5dc5d1

Browse files
authored
Bump astroid version, pylint version and drop our f-string workaround (#2746)
## Changes Our code works around a limitation of astroid < 3.3 where f-strings are not inferred This PR: - updates pylint and astroid - drops workarounds - fixes corresponding tests ### Linked issues None ### Functionality None ### Tests - [x] updated unit tests --------- Co-authored-by: Eric Vergnaud <eric.vergnaud@databricks.com>
1 parent bb696d3 commit f5dc5d1

File tree

7 files changed

+15
-58
lines changed

7 files changed

+15
-58
lines changed

pyproject.toml

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ dependencies = ["databricks-sdk~=0.30",
4949
"databricks-labs-blueprint>=0.8,<0.9",
5050
"PyYAML>=6.0.0,<7.0.0",
5151
"sqlglot>=25.5.0,<25.23",
52-
"astroid>=3.2.2"]
52+
"astroid>=3.3.1"]
5353

5454
[project.optional-dependencies]
5555
pylsp = [
@@ -74,7 +74,7 @@ dependencies = [
7474
"black~=24.3.0",
7575
"coverage[toml]~=7.4.4",
7676
"mypy~=1.9.0",
77-
"pylint~=3.2.2",
77+
"pylint~=3.3.1",
7878
"pylint-pytest==2.0.0a0",
7979
"databricks-labs-pylint~=0.4.0",
8080
"databricks-labs-pytester>=0.2.1",
@@ -209,7 +209,7 @@ fail-under = 10.0
209209
# ignore-list. The regex matches against paths and can be in Posix or Windows
210210
# format. Because '\\' represents the directory delimiter on Windows systems, it
211211
# can't be used as an escape character.
212-
ignore-paths='^tests/unit/source_code/samples/.*$'
212+
ignore-paths='^tests/unit/source_code/samples/.*$'
213213

214214
# Files or directories matching the regular expression patterns are skipped. The
215215
# regex matches against base names, not paths. The default value ignores Emacs
@@ -587,7 +587,10 @@ disable = [
587587
"fixme",
588588
"consider-using-assignment-expr",
589589
"logging-fstring-interpolation",
590-
"consider-using-any-or-all"
590+
"consider-using-any-or-all",
591+
"too-many-positional-arguments",
592+
"unnecessary-default-type-args",
593+
"logging-not-lazy"
591594
]
592595

593596
# Enable the message, report, category or checker with the given id(s). You can

src/databricks/labs/ucx/source_code/python/python_infer.py

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,6 @@ def _infer_values(cls, node: NodeNG) -> Iterator[Iterable[NodeNG]]:
6464
# deal with node types that don't implement 'inferred()'
6565
if node is Uninferable or isinstance(node, Const):
6666
yield [node]
67-
elif isinstance(node, JoinedStr):
68-
yield from cls._infer_values_from_joined_string(node)
69-
elif isinstance(node, FormattedValue):
70-
yield from _LocalInferredValue.do_infer_values(node.value)
7167
else:
7268
yield from cls._safe_infer_internal(node)
7369

@@ -91,20 +87,6 @@ def _unsafe_infer_internal(cls, node: NodeNG):
9187
continue
9288
yield from _LocalInferredValue.do_infer_values(inferred)
9389

94-
@classmethod
95-
def _infer_values_from_joined_string(cls, node: NodeNG) -> Iterator[Iterable[NodeNG]]:
96-
assert isinstance(node, JoinedStr)
97-
yield from cls._infer_values_from_joined_values(node.values)
98-
99-
@classmethod
100-
def _infer_values_from_joined_values(cls, nodes: list[NodeNG]) -> Iterator[Iterable[NodeNG]]:
101-
if len(nodes) == 1:
102-
yield from _LocalInferredValue.do_infer_values(nodes[0])
103-
return
104-
for firsts in _LocalInferredValue.do_infer_values(nodes[0]):
105-
for remains in cls._infer_values_from_joined_values(nodes[1:]):
106-
yield list(firsts) + list(remains)
107-
10890
def __init__(self, atoms: Iterable[NodeNG]):
10991
self._atoms = list(atoms)
11092

tests/unit/source_code/python/test_python_infer.py

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -88,22 +88,6 @@ def test_infers_fstring_values():
8888
assert strings == ["Hello abc, ghi!", "Hello abc, jkl!", "Hello def, ghi!", "Hello def, jkl!"]
8989

9090

91-
def test_fails_to_infer_cascading_fstring_values():
92-
# The purpose of this test is to detect a change in astroid support for f-strings
93-
source = """
94-
value1 = "John"
95-
value2 = f"Hello {value1}"
96-
value3 = f"{value2}, how are you today?"
97-
"""
98-
tree = Tree.parse(source)
99-
nodes = tree.locate(Assign, [])
100-
tree = Tree(nodes[2].value) # value of value3 = ...
101-
values = list(InferredValue.infer_from_node(tree.node))
102-
# for now, we simply check failure to infer!
103-
assert any(not value.is_inferred() for value in values)
104-
# the expected value would be ["Hello John, how are you today?"]
105-
106-
10791
def test_infers_externally_defined_value():
10892
state = CurrentSessionState()
10993
state.named_parameters = {"my-widget": "my-value"}

tests/unit/source_code/samples/functional/file-access/direct-fs2.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,9 @@
66
print(len(result))
77
index = 10
88
spark.sql(f"SELECT * FROM table_{index}").collect()
9-
# ucx[cannot-autofix-table-reference:+2:0:+2:40] Can't migrate table_name argument in 'f'SELECT * FROM {table_name}'' because its value cannot be computed
109
table_name = f"table_{index}"
1110
spark.sql(f"SELECT * FROM {table_name}").collect()
12-
# ucx[table-migrated-to-uc:+4:4:+4:20] Table old.things is migrated to brand.new.stuff in Unity Catalog
13-
# ucx[cannot-autofix-table-reference:+3:4:+3:20] Can't migrate table_name argument in 'query' because its value cannot be computed
11+
# ucx[table-migrated-to-uc:+3:4:+3:20] Table old.things is migrated to brand.new.stuff in Unity Catalog
1412
table_name = f"table_{index}"
1513
for query in ["SELECT * FROM old.things", f"SELECT * FROM {table_name}"]:
1614
spark.sql(query).collect()

tests/unit/source_code/samples/sys-path-with-fstring.py renamed to tests/unit/source_code/samples/sys-path-with-uninferrable.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import sys
22

33
name_1 = "whatever"
4-
name_2 = f"{name_1}"
4+
name_2 = some_call(name_1)
55
sys.path.append(f"{name_2}")
66
names = [f"{name_2}", name_2]
77
for name in names:

tests/unit/source_code/test_dependencies.py

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -199,24 +199,15 @@ def load_dependency(self, path_lookup: PathLookup, dependency: Dependency) -> So
199199
]
200200

201201

202-
def test_dependency_resolver_raises_problem_for_non_inferable_sys_path(simple_dependency_resolver):
202+
def test_dependency_resolver_raises_problem_for_uninferrable_sys_path(simple_dependency_resolver):
203203
maybe = simple_dependency_resolver.build_local_file_dependency_graph(
204-
Path("sys-path-with-fstring.py"), CurrentSessionState()
204+
Path("sys-path-with-uninferrable.py"), CurrentSessionState()
205205
)
206206
assert list(maybe.problems) == [
207-
DependencyProblem(
208-
code='sys-path-cannot-compute-value',
209-
message="Can't update sys.path from f'{name_2}' because the expression cannot be computed",
210-
source_path=Path('sys-path-with-fstring.py'),
211-
start_line=4,
212-
start_col=16,
213-
end_line=4,
214-
end_col=27,
215-
),
216207
DependencyProblem(
217208
code='sys-path-cannot-compute-value',
218209
message="Can't update sys.path from name because the expression cannot be computed",
219-
source_path=Path('sys-path-with-fstring.py'),
210+
source_path=Path('sys-path-with-uninferrable.py'),
220211
start_line=7,
221212
start_col=20,
222213
end_line=7,

tests/unit/source_code/test_notebook.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ def test_notebook_builds_python_dependency_graph_with_fstring_loop(mock_path_loo
249249
container = maybe.dependency.load(mock_path_lookup)
250250
assert container is not None
251251
container.build_dependency_graph(graph)
252-
expected_paths = [path, "leaf1.py", "leaf3.py"]
252+
expected_paths = [path, "leaf1.py", "leaf2.py", "leaf3.py"]
253253
all_paths = set(d.path for d in graph.all_dependencies)
254254
assert all_paths == {mock_path_lookup.cwd / path for path in expected_paths}
255255

@@ -279,13 +279,12 @@ def test_does_not_detect_partial_call_to_dbutils_notebook_run_in_python_code_()
279279
assert len(nodes) == 0
280280

281281

282-
def test_raises_advice_when_dbutils_notebook_run_is_too_complex() -> None:
282+
def test_lints_complex_dbutils_notebook_run() -> None:
283283
source = """
284284
name1 = "John"
285285
name2 = f"{name1}"
286286
dbutils.notebook.run(f"Hey {name2}")
287287
"""
288288
linter = DbutilsPyLinter(CurrentSessionState())
289289
advices = list(linter.lint(source))
290-
assert len(advices) == 1
291-
assert advices[0].code == "notebook-run-cannot-compute-value"
290+
assert not advices

0 commit comments

Comments
 (0)