Copilot commented on code in PR #1662:
URL:
https://github.com/apache/datafusion-python/pull/1662#discussion_r3706819899
##########
python/datafusion/expr.py:
##########
@@ -90,6 +95,25 @@
CreateCatalog = expr_internal.CreateCatalog
CreateCatalogSchema = expr_internal.CreateCatalogSchema
CreateExternalTable = expr_internal.CreateExternalTable
+
+
+@deprecated("CreateExternalTable.location() is deprecated; use locations()
instead.")
+def _create_external_table_location(self: Any) -> str:
Review Comment:
Returning an empty string when `locations()` is empty is ambiguous (it’s
indistinguishable from a legitimate empty location string) and may hide a real
issue. Consider raising a clear exception when there are no locations, or
change the shim to return `None` (and document/annotate it as `str | None`) if
an empty set of locations is valid.
##########
python/datafusion/expr.py:
##########
@@ -90,6 +95,25 @@
CreateCatalog = expr_internal.CreateCatalog
CreateCatalogSchema = expr_internal.CreateCatalogSchema
CreateExternalTable = expr_internal.CreateExternalTable
+
+
+@deprecated("CreateExternalTable.location() is deprecated; use locations()
instead.")
+def _create_external_table_location(self: Any) -> str:
+ """Return the first external table location.
+
+ Examples:
+ >>> class Command:
+ ... def locations(self) -> list[str]:
+ ... return ["data.csv"]
+ >>> _create_external_table_location(Command())
+ 'data.csv'
+ """
+ locations = self.locations()
+ return locations[0] if locations else ""
Review Comment:
Returning an empty string when `locations()` is empty is ambiguous (it’s
indistinguishable from a legitimate empty location string) and may hide a real
issue. Consider raising a clear exception when there are no locations, or
change the shim to return `None` (and document/annotate it as `str | None`) if
an empty set of locations is valid.
##########
Cargo.toml:
##########
@@ -72,3 +72,13 @@ codegen-units = 2
# We cannot publish to crates.io with any patches in the below section.
Developers
# must remove any entries in this section before creating a release candidate.
[patch.crates-io]
+datafusion = { git = "https://github.com/apache/datafusion";, rev =
"dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
+datafusion-substrait = { git = "https://github.com/apache/datafusion";, rev =
"dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
+datafusion-proto = { git = "https://github.com/apache/datafusion";, rev =
"dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
+datafusion-ffi = { git = "https://github.com/apache/datafusion";, rev =
"dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
+datafusion-catalog = { git = "https://github.com/apache/datafusion";, rev =
"dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
+datafusion-common = { git = "https://github.com/apache/datafusion";, rev =
"dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
+datafusion-functions-aggregate = { git =
"https://github.com/apache/datafusion";, rev =
"dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
+datafusion-functions-window = { git = "https://github.com/apache/datafusion";,
rev = "dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
+datafusion-spark = { git = "https://github.com/apache/datafusion";, rev =
"dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
+datafusion-expr = { git = "https://github.com/apache/datafusion";, rev =
"dbcb5c0f729e9ef6b0ab4c79253fe3b657929f48" }
Review Comment:
Committing `[patch.crates-io]` git overrides forces consumers/CI to fetch
from GitHub and pins builds to a specific commit, which can reduce reliability
(network outages, repo unavailability) and complicate supply-chain/repro
requirements. If the intent is temporary tracking of upstream main, consider
gating this behind a feature or using a separate dev-only mechanism (and/or add
a clear release checklist/CI guard to ensure patches are removed before
publishing).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
