Skip to content

Commit a889b0a

Browse files
authored
Merge pull request #189 from decargroup/bugfix/188-some-tests-failing-when-running-locally-in-python-313-environment
Bugfix/188 some tests failing when running locally in python 313 environment
2 parents 2215358 + 1880bda commit a889b0a

File tree

13 files changed

+144
-73
lines changed

13 files changed

+144
-73
lines changed

.github/workflows/test-package-no-mosek.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ jobs:
1010
strategy:
1111
fail-fast: false
1212
matrix:
13-
python-version: ['3.8', '3.9', '3.10', '3.11']
13+
python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']
1414
steps:
1515
- uses: actions/checkout@v2
1616
- name: Set up Python ${{matrix.python-version}}

.github/workflows/test-package.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ jobs:
1010
strategy:
1111
fail-fast: false
1212
matrix:
13-
python-version: ['3.12']
13+
python-version: ['3.13']
1414
steps:
1515
- uses: actions/checkout@v2
1616
- name: Set up Python ${{matrix.python-version}}

pykoop/_sklearn_metaestimators/metaestimators.py

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ class to do so. My compromise was to copy and adjust that code for my own uses,
1515
"""
1616

1717
import abc
18+
from contextlib import suppress
1819
from typing import Any, Dict, List
1920

2021
import numpy as np
@@ -28,10 +29,18 @@ def _get_params(self, attr: str, deep: bool = True) -> Dict[str, Any]:
2829
out = super().get_params(deep=deep)
2930
if not deep:
3031
return out
32+
3133
estimators = getattr(self, attr)
32-
if not hasattr(estimators, '__iter__'):
34+
try:
35+
out.update(estimators)
36+
except (TypeError, ValueError):
37+
# Ignore TypeError for cases where estimators is not a list of
38+
# (name, estimator) and ignore ValueError when the list is not
39+
# formatted correctly. This is to prevent errors when calling
40+
# `set_params`. `BaseEstimator.set_params` calls `get_params` which
41+
# can error for invalid values for `estimators`.
3342
return out
34-
out.update(estimators)
43+
3544
for name, estimator in estimators:
3645
if hasattr(estimator, "get_params"):
3746
for key, value in estimator.get_params(deep=True).items():
@@ -43,14 +52,18 @@ def _set_params(self, attr: str, **params) -> '_BaseComposition':
4352
# 1. All steps
4453
if attr in params:
4554
setattr(self, attr, params.pop(attr))
46-
# 2. Step replacement
55+
# 2. Replace items with estimators in params
4756
items = getattr(self, attr)
48-
names = []
49-
if hasattr(items, '__iter__'):
50-
names, _ = zip(*items)
51-
for name in list(params.keys()):
52-
if "__" not in name and name in names:
53-
self._replace_estimator(attr, name, params.pop(name))
57+
if isinstance(items, list) and items:
58+
# Get item names used to identify valid names in params
59+
# `zip` raises a TypeError when `items` does not contains
60+
# elements of length 2
61+
with suppress(TypeError):
62+
item_names, _ = zip(*items)
63+
for name in list(params.keys()):
64+
if "__" not in name and name in item_names:
65+
self._replace_estimator(attr, name, params.pop(name))
66+
5467
# 3. Step parameters and other initialisation arguments
5568
super().set_params(**params)
5669
return self
@@ -68,11 +81,11 @@ def _validate_names(self, names: List[str]) -> None:
6881
if len(set(names)) != len(names):
6982
raise ValueError("Names provided are not unique: {0!r}".format(
7083
list(names)))
71-
conflict_names = set(names).intersection(self.get_params(deep=False))
72-
if conflict_names:
84+
invalid_names = set(names).intersection(self.get_params(deep=False))
85+
if invalid_names:
7386
raise ValueError(
7487
"Estimator names conflict with constructor arguments: {0!r}".
75-
format(sorted(conflict_names)))
88+
format(sorted(invalid_names)))
7689
invalid_names = [name for name in names if "__" in name]
7790
if invalid_names:
7891
raise ValueError(

pykoop/centers.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ class GridCenters(Centers):
7878
>>> grid.fit(X_msd[:, 1:]) # Remove episode feature
7979
GridCenters(n_points_per_feature=4)
8080
>>> grid.centers_
81-
array([...])
81+
array(...)
8282
"""
8383

8484
def __init__(
@@ -155,7 +155,7 @@ class UniformRandomCenters(Centers):
155155
>>> rand.fit(X_msd[:, 1:]) # Remove episode feature
156156
UniformRandomCenters(n_centers=10)
157157
>>> rand.centers_
158-
array([...])
158+
array(...)
159159
"""
160160

161161
def __init__(
@@ -235,7 +235,7 @@ class GaussianRandomCenters(Centers):
235235
>>> rand.fit(X_msd[:, 1:]) # Remove episode feature
236236
GaussianRandomCenters(n_centers=10)
237237
>>> rand.centers_
238-
array([...])
238+
array(...)
239239
"""
240240

241241
def __init__(
@@ -306,15 +306,15 @@ class QmcCenters(Centers):
306306
>>> qmc.fit(X_msd[:, 1:]) # Remove episode feature
307307
QmcCenters(n_centers=10)
308308
>>> qmc.centers_
309-
array([...])
309+
array(...)
310310
311311
Generate centers using a Sobol sequence
312312
313313
>>> qmc = pykoop.QmcCenters(n_centers=8, qmc=scipy.stats.qmc.Sobol)
314314
>>> qmc.fit(X_msd[:, 1:]) # Remove episode feature
315315
QmcCenters(n_centers=8, qmc=<class 'scipy.stats._qmc.Sobol'>)
316316
>>> qmc.centers_
317-
array([...])
317+
array(...)
318318
"""
319319

320320
def __init__(
@@ -430,7 +430,7 @@ class ClusterCenters(Centers):
430430
>>> kmeans.fit(X_msd[:, 1:]) # Remove episode feature
431431
ClusterCenters(estimator=KMeans(n_clusters=3))
432432
>>> kmeans.centers_
433-
array([...])
433+
array(...)
434434
"""
435435

436436
def __init__(
@@ -507,7 +507,7 @@ class GaussianMixtureRandomCenters(Centers):
507507
>>> gmm.fit(X_msd[:, 1:]) # Remove episode feature
508508
GaussianMixtureRandomCenters(estimator=GaussianMixture(n_components=3))
509509
>>> gmm.centers_
510-
array([...])
510+
array(...)
511511
"""
512512

513513
def __init__(

pykoop/kernel_approximation.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class RandomFourierKernelApprox(KernelApproximation):
113113
>>> ka.fit(X_msd[:, 1:]) # Remove episode feature
114114
RandomFourierKernelApprox(n_components=10, random_state=1234)
115115
>>> ka.transform(X_msd[:, 1:])
116-
array([...])
116+
array(...)
117117
"""
118118

119119
_ft_lookup = {
@@ -267,6 +267,10 @@ def transform(self, X: np.ndarray) -> np.ndarray:
267267
"""
268268
sklearn.utils.validation.check_is_fitted(self)
269269
X = sklearn.utils.validation.check_array(X)
270+
if X.shape[1] != self.n_features_in_:
271+
raise ValueError(
272+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
273+
f"is expecting {self.n_features_in_} features as input.")
270274
X_scaled = np.sqrt(2 * self.shape) * X
271275
products = X_scaled @ self.random_weights_ # (n_samples, n_components)
272276
if self.method == 'weight_only':
@@ -314,7 +318,7 @@ class RandomBinningKernelApprox(KernelApproximation):
314318
>>> ka.fit(X_msd[:, 1:]) # Remove episode feature
315319
RandomBinningKernelApprox(n_components=10, random_state=1234)
316320
>>> ka.transform(X_msd[:, 1:])
317-
array([...])
321+
array(...)
318322
"""
319323

320324
_ddot_lookup = {
@@ -451,6 +455,10 @@ def transform(self, X: np.ndarray) -> np.ndarray:
451455
"""
452456
sklearn.utils.validation.check_is_fitted(self)
453457
X = sklearn.utils.validation.check_array(X)
458+
if X.shape[1] != self.n_features_in_:
459+
raise ValueError(
460+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
461+
f"is expecting {self.n_features_in_} features as input.")
454462
X_scaled = np.sqrt(2 * self.shape) * X
455463
X_hashed = self._hash_samples(X_scaled)
456464
Xt = self.encoder_.transform(X_hashed) / np.sqrt(self.n_components)

pykoop/koopman_pipeline.py

Lines changed: 51 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ def fit(
721721
}
722722
# Validate data
723723
X = sklearn.utils.validation.check_array(X, **self._check_array_params)
724-
# Set numbre of input features (including episode feature)
724+
# Set number of input features (including episode feature)
725725
self.n_features_in_ = X.shape[1]
726726
# Extract episode feature
727727
if self.episode_feature_:
@@ -751,12 +751,11 @@ def transform(self, X: np.ndarray) -> np.ndarray:
751751
X,
752752
**self._check_array_params,
753753
)
754-
# Check input shape
754+
# Check number of features
755755
if X.shape[1] != self.n_features_in_:
756-
raise ValueError(f'{self.__class__.__name__} `fit()` called '
757-
f'with {self.n_features_in_} features, but '
758-
f'`transform()` called with {X.shape[1]} '
759-
'features.')
756+
raise ValueError(
757+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
758+
f"is expecting {self.n_features_in_} features as input.")
760759
return self._apply_transform_or_inverse(X, 'transform')
761760

762761
def inverse_transform(self, X: np.ndarray) -> np.ndarray:
@@ -996,12 +995,11 @@ def transform(self, X: np.ndarray) -> np.ndarray:
996995
X,
997996
**self._check_array_params,
998997
)
999-
# Check input shape
998+
# Check number of features
1000999
if X.shape[1] != self.n_features_in_:
1001-
raise ValueError(f'{self.__class__.__name__} `fit()` called '
1002-
f'with {self.n_features_in_} features, but '
1003-
f'`transform()` called with {X.shape[1]} '
1004-
'features.')
1000+
raise ValueError(
1001+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
1002+
f"is expecting {self.n_features_in_} features as input.")
10051003
return self._apply_transform_or_inverse(X, 'transform')
10061004

10071005
def inverse_transform(self, X: np.ndarray) -> np.ndarray:
@@ -1014,12 +1012,11 @@ def inverse_transform(self, X: np.ndarray) -> np.ndarray:
10141012
X,
10151013
**self._check_array_params,
10161014
)
1017-
# Check input shape
1015+
# Check number of features
10181016
if X.shape[1] != self.n_features_out_:
1019-
raise ValueError(f'{self.__class__.__name__} `fit()` output '
1020-
f'{self.n_features_out_} features, but '
1021-
'`inverse_transform()` called with '
1022-
f'{X.shape[1]} features.')
1017+
raise ValueError(
1018+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
1019+
f"is expecting {self.n_features_out_} features as input.")
10231020
return self._apply_transform_or_inverse(X, 'inverse_transform')
10241021

10251022
def _apply_transform_or_inverse(self, X: np.ndarray,
@@ -1270,6 +1267,11 @@ def predict(self, X: np.ndarray) -> np.ndarray:
12701267
self._validate_feature_names(X)
12711268
# Validate array
12721269
X = sklearn.utils.validation.check_array(X, **self._check_array_params)
1270+
# Check number of features
1271+
if X.shape[1] != self.n_features_in_:
1272+
raise ValueError(
1273+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
1274+
f"is expecting {self.n_features_in_} features as input.")
12731275
# Split episodes
12741276
episodes = split_episodes(X, episode_feature=self.episode_feature_)
12751277
# Predict for each episode
@@ -1623,8 +1625,13 @@ def _validate_feature_names(self, X: np.ndarray) -> None:
16231625
if not np.all(_extract_feature_names(X) == self.feature_names_in_):
16241626
raise ValueError('Input features do not match fit features.')
16251627

1626-
# Extra estimator tags
1627-
# https://scikit-learn.org/stable/developers/develop.html#estimator-tags
1628+
def __sklearn_tags__(self):
1629+
tags = super().__sklearn_tags__()
1630+
tags.target_tags.required = False
1631+
tags.target_tags.single_output = False
1632+
tags.target_tags.multi_output = True
1633+
return tags
1634+
16281635
def _more_tags(self):
16291636
return {
16301637
'multioutput': True,
@@ -1812,12 +1819,11 @@ def transform(self, X: np.ndarray) -> np.ndarray:
18121819
X,
18131820
**self._check_array_params,
18141821
)
1815-
# Check input shape
1822+
# Check number of features
18161823
if X.shape[1] != self.n_features_in_:
1817-
raise ValueError(f'{self.__class__.__name__} `fit()` called '
1818-
f'with {self.n_features_in_} features, but '
1819-
f'`transform()` called with {X.shape[1]} '
1820-
'features.')
1824+
raise ValueError(
1825+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
1826+
f"is expecting {self.n_features_in_} features as input.")
18211827
# Split episodes
18221828
episodes = split_episodes(X, episode_feature=self.episode_feature_)
18231829
episodes_state = []
@@ -1873,12 +1879,11 @@ def inverse_transform(self, X: np.ndarray) -> np.ndarray:
18731879
sklearn.utils.validation.check_is_fitted(self)
18741880
X = sklearn.utils.validation.check_array(
18751881
X, **self._check_array_params)
1876-
# Check input shape
1882+
# Check number of features
18771883
if X.shape[1] != self.n_features_out_:
1878-
raise ValueError(f'{self.__class__.__name__} `fit()` output '
1879-
f'{self.n_features_out_} features, but '
1880-
'`inverse_transform()` called with '
1881-
f'{X.shape[1]} features.')
1884+
raise ValueError(
1885+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
1886+
f"is expecting {self.n_features_out_} features as input.")
18821887
# Split episodes
18831888
episodes = split_episodes(X, episode_feature=self.episode_feature_)
18841889
episodes_state = []
@@ -2278,12 +2283,11 @@ def transform(self, X: np.ndarray) -> np.ndarray:
22782283
X,
22792284
**self._check_array_params,
22802285
)
2281-
# Check input shape
2286+
# Check number of features
22822287
if X.shape[1] != self.n_features_in_:
2283-
raise ValueError(f'{self.__class__.__name__} `fit()` called '
2284-
f'with {self.n_features_in_} features, but '
2285-
f'`transform()` called with {X.shape[1]} '
2286-
'features.')
2288+
raise ValueError(
2289+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
2290+
f"is expecting {self.n_features_in_} features as input.")
22872291
# Apply lifting functions
22882292
X_out = X
22892293
for _, lf in self.lifting_functions_:
@@ -2309,12 +2313,11 @@ def inverse_transform(self, X: np.ndarray) -> np.ndarray:
23092313
X,
23102314
**self._check_array_params,
23112315
)
2312-
# Check input shape
2316+
# Check number of features
23132317
if X.shape[1] != self.n_features_out_:
2314-
raise ValueError(f'{self.__class__.__name__} `fit()` output '
2315-
f'{self.n_features_out_} features, but '
2316-
'`inverse_transform()` called with '
2317-
f'{X.shape[1]} features.')
2318+
raise ValueError(
2319+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
2320+
f"is expecting {self.n_features_out_} features as input.")
23182321
# Apply inverse lifting functions in reverse order
23192322
X_out = X
23202323
for _, lf in self.lifting_functions_[::-1]:
@@ -2360,6 +2363,11 @@ def predict(self, X: np.ndarray) -> np.ndarray:
23602363
X,
23612364
**self._check_array_params,
23622365
)
2366+
# Check number of features
2367+
if X.shape[1] != self.n_features_in_:
2368+
raise ValueError(
2369+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
2370+
f"is expecting {self.n_features_in_} features as input.")
23632371
# Lift data matrix
23642372
X_trans = self.transform(X)
23652373
# Predict in lifted space
@@ -2405,6 +2413,11 @@ def score(self, X: np.ndarray, y: Optional[np.ndarray] = None) -> float:
24052413
self._validate_feature_names(X)
24062414
# Validate input array
24072415
X = sklearn.utils.validation.check_array(X, **self._check_array_params)
2416+
# Check number of features
2417+
if X.shape[1] != self.n_features_in_:
2418+
raise ValueError(
2419+
f"X has {X.shape[1]} features, but {self.__class__.__name__} "
2420+
f"is expecting {self.n_features_in_} features as input.")
24082421
scorer = KoopmanPipeline.make_scorer()
24092422
score = scorer(self, X, None)
24102423
return score

pykoop/lifting_functions.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ class SkLearnLiftingFn(koopman_pipeline.EpisodeIndependentLiftingFn):
7272
['ep', 'StandardScaler(x0)', 'StandardScaler(x1)', 'StandardScaler(u0)']
7373
>>> X_msd_pp = std_scaler.transform(X_msd)
7474
>>> np.mean(X_msd_pp[:, 1:], axis=0)
75-
array([...])
75+
array(...)
7676
>>> np.std(X_msd_pp[:, 1:], axis=0)
77-
array([...])
77+
array(...)
7878
"""
7979

8080
def __init__(

0 commit comments

Comments
 (0)