bearomorphism opened a new pull request, #59369:
URL: https://github.com/apache/spark/pull/59369
### What changes were proposed in this pull request?
Make `_CrossValidatorReadWrite` inherit `_ValidatorParams` in
`pyspark.ml.connect.tuning`, declaring the parameter dependency already used by
its save/load implementation. Remove four now-unnecessary `type:
ignore[attr-defined]` comments.
Both `CrossValidator` and `CrossValidatorModel` already inherit
`_ValidatorParams` through `_CrossValidatorParams`. This adds an explicit path
from the persistence mixin to the same shared ancestor, rather than relying on
a sibling base to supply its parameters and getters.
The two suppressions for estimator/evaluator persistence methods remain:
those methods are not declared on the estimator/evaluator base types, which is
a separate issue.
### Why are the changes needed?
Removing the suppression from `self.getEstimatorParamMaps()` without
declaring the dependency produces:
```text
"_CrossValidatorReadWrite" has no attribute "getEstimatorParamMaps"
[attr-defined]
```
The concrete classes provide this getter at runtime, but mypy checks the
persistence mixin against its own declared bases. Inheriting the existing
parameter class makes that requirement explicit and allows mypy to check the
parameter accesses.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
No new test files were added. The missing-attribute error was reproduced
with mypy using a temporary shadow copy with the target suppression removed.
The changed module then passed:
```bash
source .venv/bin/activate
python -m mypy --config-file python/mypy.ini --follow-imports=silent \
python/pyspark/ml/connect/tuning.py
python -m ruff check python/pyspark/ml/connect/tuning.py
python -m ruff format --check python/pyspark/ml/connect/tuning.py
git diff --check
```
Compared the concrete classes' method resolution orders before and after the
change. The relevant getters, persistence methods, and parameter-storage
methods still resolve to the same implementations. The initialization ordering
change swaps independent seed/parallelism default assignments.
Also ran the following standalone local-persistence smoke checks, without a
Spark session or JVM. These cover both concrete classes, parameter maps,
defaults, parameter ownership, copying, and model metadata. The Spark-backed
test suites were not run.
<details>
<summary>Reproduce the local-persistence checks</summary>
```bash
PYTHONPATH=python .venv/bin/python - <<'PY'
import tempfile
from pathlib import Path
from pyspark.ml.connect.classification import LogisticRegression
from pyspark.ml.connect.evaluation import BinaryClassificationEvaluator
from pyspark.ml.connect.pipeline import PipelineModel
from pyspark.ml.connect.tuning import CrossValidator, CrossValidatorModel
from pyspark.ml.tuning import ParamGridBuilder
estimator = LogisticRegression(maxIter=2)
evaluator = BinaryClassificationEvaluator()
param_maps = ParamGridBuilder().addGrid(estimator.maxIter, [2, 3]).build()
cv = CrossValidator(estimator=estimator, evaluator=evaluator,
estimatorParamMaps=param_maps)
model = cv._copyValues(CrossValidatorModel(PipelineModel([]), [0.7, 0.8],
[0.1, 0.2]))
for original in (cv, model):
cls = type(original)
with tempfile.TemporaryDirectory() as tmp:
path = str(Path(tmp) / 'saved')
original.saveToLocal(path)
loaded = cls.loadFromLocal(path)
assert type(loaded) is cls
assert loaded.uid == original.uid
assert loaded.getEstimatorParamMaps() == param_maps
assert loaded.getEstimator().uid == estimator.uid
assert loaded.getEvaluator().uid == evaluator.uid
assert loaded.getNumFolds() == original.getNumFolds() == 3
assert loaded.getFoldCol() == original.getFoldCol() == ''
assert loaded.getSeed() == original.getSeed()
assert all(p.parent == loaded.uid for p in loaded.params)
copied = loaded.copy()
assert copied.getEstimatorParamMaps() == param_maps
if isinstance(loaded, CrossValidatorModel):
assert loaded.avgMetrics == original.avgMetrics
assert loaded.stdMetrics == original.stdMetrics
assert loaded.bestModel.uid == original.bestModel.uid
else:
assert loaded.getParallelism() == 1
print(f'{cls.__name__}: local persistence, defaults, parameter
ownership, and copy passed')
PY
```
</details>
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Pi 1.1.0
--
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]
