refactor(tidy3d): FXC-4683-move-config-to-new-common-submodule #3124
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marcorudolphflex
requested review from
daquinteroflex and
yaugenst-flex
as code owners
marcorudolphflex
force-pushed
the
FXC-4683-move-config-to-new-common-submodule
branch
from
83eaf08 to
a09ca37
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
marcorudolphflex
force-pushed
the
FXC-4683-move-config-to-new-common-submodule
branch
from
a09ca37 to
9c6c053
Compare
Contributor
Diff CoverageDiff: origin/develop...HEAD, staged and unstaged changes
Summary
tidy3d/_common/config/init.pyLines 61-74 61 global _base_manager, Env
62 if _base_manager is not None:
63 try:
64 _base_manager.apply_web_env({})
! 65 except AttributeError:
! 66 pass
67 _base_manager = ConfigManager(profile=profile)
68 _config_wrapper.reset_manager(_base_manager)
69 if Env is None:
! 70 initialize_env()
71 Env.reset_manager(_base_manager)
72 return _config_wrapper
73 Lines 78-86 78 return _base_manager
79
80
81 def __getattr__(name: str) -> Any:
! 82 if name == "Env":
! 83 initialize_env()
! 84 return Env
! 85 return getattr(config, name)tidy3d/_common/config/legacy.pyLines 36-44 36 self._frozen = False # retained for backwards compatibility tests
37
38 @property
39 def logging_level(self) -> LogLevel:
! 40 return self._manager.get_section("logging").level
41
42 @logging_level.setter
43 def logging_level(self, value: LogLevel) -> None:
44 from warnings import warnLines 51-59 51 self._manager.update_section("logging", level=value)
52
53 @property
54 def log_suppression(self) -> bool:
! 55 return self._manager.get_section("logging").suppression
56
57 @log_suppression.setter
58 def log_suppression(self, value: bool) -> None:
59 from warnings import warnLines 66-95 66 self._manager.update_section("logging", suppression=value)
67
68 @property
69 def use_local_subpixel(self) -> Optional[bool]:
! 70 return self._manager.get_section("simulation").use_local_subpixel
71
72 @use_local_subpixel.setter
73 def use_local_subpixel(self, value: Optional[bool]) -> None:
! 74 from warnings import warn
75
! 76 warn(
77 "'config.use_local_subpixel' is deprecated; use 'config.simulation.use_local_subpixel'.",
78 DeprecationWarning,
79 stacklevel=2,
80 )
! 81 self._manager.update_section("simulation", use_local_subpixel=value)
82
83 @property
84 def suppress_rf_license_warning(self) -> bool:
! 85 return self._manager.get_section("microwave").suppress_rf_license_warning
86
87 @suppress_rf_license_warning.setter
88 def suppress_rf_license_warning(self, value: bool) -> None:
! 89 from warnings import warn
90
! 91 warn(
92 "'config.suppress_rf_license_warning' is deprecated; "
93 "use 'config.microwave.suppress_rf_license_warning'.",
94 DeprecationWarning,
95 stacklevel=2,Lines 93-112 93 "use 'config.microwave.suppress_rf_license_warning'.",
94 DeprecationWarning,
95 stacklevel=2,
96 )
! 97 self._manager.update_section("microwave", suppress_rf_license_warning=value)
98
99 @property
100 def frozen(self) -> bool:
! 101 return self._frozen
102
103 @frozen.setter
104 def frozen(self, value: bool) -> None:
! 105 self._frozen = bool(value)
106
107 def save(self, include_defaults: bool = False) -> None:
! 108 self._manager.save(include_defaults=include_defaults)
109
110 def reset_manager(self, manager: ConfigManager) -> None:
111 """Swap the underlying manager instance."""Lines 118-127 118 normalized = normalize_profile_name(profile)
119 self._manager.switch_profile(normalized)
120 try:
121 from tidy3d._common.config import Env as _legacy_env
! 122 except Exception:
! 123 _legacy_env = None
124 if _legacy_env is not None:
125 _legacy_env._sync_to_manager(apply_env=True)
126
127 def __getattr__(self, name: str) -> Any:Lines 139-147 139 }:
140 prop = getattr(type(self), name)
141 prop.fset(self, value)
142 else:
! 143 setattr(self._manager, name, value)
144
145 def __str__(self) -> str:
146 return self._manager.format()Lines 164-172 164 env_vars: Optional[dict[str, str]] = None,
165 environment: Optional[LegacyEnvironment] = None,
166 ) -> None:
167 if name is None:
! 168 raise ValueError("Environment name is required")
169 self._manager = manager
170 self._name = normalize_profile_name(name)
171 self._environment = environment
172 self._pending: dict[str, Any] = {}Lines 176-190 176 self._pending["website_endpoint"] = website_endpoint
177 if s3_region is not None:
178 self._pending["s3_region"] = s3_region
179 if ssl_verify is not None:
! 180 self._pending["ssl_verify"] = ssl_verify
181 if enable_caching is not None:
! 182 self._pending["enable_caching"] = enable_caching
183 if ssl_version is not None:
! 184 self._pending["ssl_version"] = ssl_version
185 if env_vars is not None:
! 186 self._pending["env_vars"] = dict(env_vars)
187
188 def reset_manager(self, manager: ConfigManager) -> None:
189 self._manager = managerLines 191-201 191 @property
192 def manager(self) -> Optional[ConfigManager]:
193 if self._manager is not None:
194 return self._manager
! 195 if self._environment is not None:
! 196 return self._environment._manager
! 197 return None
198
199 def active(self) -> None:
200 _warn_env_deprecated()
201 environment = self._environmentLines 199-209 199 def active(self) -> None:
200 _warn_env_deprecated()
201 environment = self._environment
202 if environment is None:
! 203 from tidy3d._common.config import Env # local import to avoid circular
204
! 205 environment = Env
206
207 environment.set_current(self)
208
209 @propertyLines 212-232 212 return _maybe_str(value)
213
214 @property
215 def website_endpoint(self) -> Optional[str]:
! 216 value = self._value("website_endpoint")
! 217 return _maybe_str(value)
218
219 @property
220 def s3_region(self) -> Optional[str]:
! 221 return self._value("s3_region")
222
223 @property
224 def ssl_verify(self) -> bool:
! 225 value = self._value("ssl_verify")
! 226 if value is None:
! 227 return True
! 228 return bool(value)
229
230 @property
231 def enable_caching(self) -> bool:
232 value = self._value("enable_caching")Lines 230-238 230 @property
231 def enable_caching(self) -> bool:
232 value = self._value("enable_caching")
233 if value is None:
! 234 return True
235 return bool(value)
236
237 @enable_caching.setter
238 def enable_caching(self, value: Optional[bool]) -> None:Lines 249-257 249 @property
250 def env_vars(self) -> dict[str, str]:
251 value = self._value("env_vars")
252 if value is None:
! 253 return {}
254 return dict(value)
255
256 @env_vars.setter
257 def env_vars(self, value: dict[str, str]) -> None:Lines 262-274 262 return self._name
263
264 @name.setter
265 def name(self, value: str) -> None:
! 266 self._name = normalize_profile_name(value)
267
268 def copy_state_from(self, other: LegacyEnvironmentConfig) -> None:
269 if not isinstance(other, LegacyEnvironmentConfig):
! 270 raise TypeError("Expected LegacyEnvironmentConfig instance.")
271 for key, value in other._pending.items():
272 if key == "env_vars" and value is not None:
273 self._pending[key] = dict(value)
274 else:Lines 274-291 274 else:
275 self._pending[key] = value
276
277 def get_real_url(self, path: str) -> str:
! 278 manager = self.manager
! 279 if manager is not None and manager.profile == self._name:
! 280 web_section = manager.get_section("web")
! 281 if hasattr(web_section, "build_api_url"):
! 282 return web_section.build_api_url(path)
283
! 284 endpoint = self.web_api_endpoint or ""
! 285 if not path:
! 286 return endpoint
! 287 return "/".join([endpoint.rstrip("/"), str(path).lstrip("/")])
288
289 def apply_pending_overrides(self) -> None:
290 manager = self.manager
291 if manager is None or manager.profile != self._name:Lines 305-324 305
306 def _web_section(self) -> dict[str, Any]:
307 manager = self.manager
308 if manager is None or WASM_BUILD:
! 309 return {}
310 profile = normalize_profile_name(self._name)
311 if manager.profile == profile:
312 section = manager.get_section("web")
313 return section.model_dump(mode="python", exclude_unset=False)
! 314 preview = manager.preview_profile(profile)
! 315 source = preview.get("web", {})
! 316 return dict(source) if isinstance(source, dict) else {}
317
318 def _value(self, key: str) -> Any:
319 if key in self._pending:
! 320 return self._pending[key]
321 return self._web_section().get(key)
322
323
324 # TODO(FXC-3827): Delete LegacyEnvironment after deprecating `tidy3d.config.Env`.Lines 385-393 385 return config
386
387 def _sync_to_manager(self, *, apply_env: bool = False) -> None:
388 if self._manager is None:
! 389 return
390 active = normalize_profile_name(self._manager.profile)
391 config = self._get_config(active)
392 config.apply_pending_overrides()
393 self._current = configLines 405-413 405
406 def _restore_env_vars(self) -> None:
407 for key, previous in self._previous_env_vars.items():
408 if previous is None:
! 409 os.environ.pop(key, None)
410 else:
411 os.environ[key] = previous
412 self._previous_env_vars = {}Lines 413-421 413
414
415 def _maybe_str(value: Any) -> Optional[str]:
416 if value is None:
! 417 return None
418 return str(value)
419
420
421 def load_legacy_flat_config(config_dir: Path) -> dict[str, Any]:Lines 439-449 439 return {}
440
441 try:
442 text = legacy_path.read_text(encoding="utf-8")
! 443 except Exception as exc:
! 444 log.warning(f"Failed to read legacy configuration file '{legacy_path}': {exc}")
! 445 return {}
446
447 try:
448 parsed = toml.loads(text)
449 except Exception as exc:Lines 521-539 521 if isinstance(values, dict):
522 manager.update_section(section, **values)
523 try:
524 manager.save(include_defaults=True)
! 525 except Exception:
! 526 if config_path.exists():
! 527 try:
! 528 config_path.unlink()
! 529 except Exception:
! 530 pass
! 531 raise
532
533 legacy_flat_path = config_dir / "config"
534 if legacy_flat_path.exists():
! 535 try:
! 536 legacy_flat_path.unlink()
! 537 except Exception as exc:
! 538 log.warning(f"Failed to remove legacy configuration file '{legacy_flat_path}': {exc}")tidy3d/_common/config/loader.pyLines 64-80 64 )
65
66 # Re-read the newly created config
67 return self._read_toml(config_path)
! 68 except Exception as exc:
! 69 log.warning(
70 f"Failed to auto-migrate legacy configuration: {exc}. "
71 "Using legacy data without migration."
72 )
! 73 return legacy
74
75 if legacy:
! 76 return legacy
77 return {}
78
79 def load_user_profile(self, profile: str) -> dict[str, Any]:
80 """Load user profile overrides (if any)."""Lines 101-112 101 """Persist profile overrides (remove file if empty)."""
102
103 profile_path = self.profile_path(profile)
104 if not data:
! 105 if profile_path.exists():
! 106 profile_path.unlink()
! 107 self._docs.pop(profile_path, None)
! 108 return
109 profile_path.parent.mkdir(mode=0o700, parents=True, exist_ok=True)
110 self._atomic_write(profile_path, data)
111
112 def profile_path(self, profile: str) -> Path:Lines 163-187 163 return {}
164
165 try:
166 text = path.read_text(encoding="utf-8")
! 167 except Exception as exc:
! 168 log.warning(f"Failed to read configuration file '{path}': {exc}")
! 169 self._docs.pop(path, None)
! 170 return {}
171
172 try:
173 document = tomlkit.parse(text)
! 174 except Exception as exc:
! 175 log.warning(f"Failed to parse configuration file '{path}': {exc}")
! 176 document = tomlkit.document()
177 self._docs[path] = document
178
179 try:
180 return toml.loads(text)
! 181 except Exception as exc:
! 182 log.warning(f"Failed to decode configuration file '{path}': {exc}")
! 183 return {}
184
185 def _atomic_write(self, path: Path, data: dict[str, Any]) -> None:
186 path.parent.mkdir(mode=0o700, parents=True, exist_ok=True)
187 tmp_dir = path.parentLines 208-224 208 tmp_path.replace(path)
209 os.chmod(path, 0o600)
210 if backup_path.exists():
211 backup_path.unlink()
! 212 except Exception:
! 213 if tmp_path.exists():
! 214 tmp_path.unlink()
! 215 if backup_path.exists():
! 216 try:
! 217 backup_path.replace(path)
! 218 except Exception:
! 219 log.warning("Failed to restore configuration backup")
! 220 raise
221
222 self._docs[path] = tomlkit.parse(toml_text)
223 Lines 227-236 227
228 overrides: dict[str, Any] = {}
229 for key, value in os.environ.items():
230 if key == "SIMCLOUD_APIKEY":
! 231 _assign_path(overrides, ("web", "apikey"), value)
! 232 continue
233 if not key.startswith("TIDY3D_"):
234 continue
235 rest = key[len("TIDY3D_") :]
236 if "__" not in rest:Lines 236-246 236 if "__" not in rest:
237 continue
238 segments = tuple(segment.lower() for segment in rest.split("__") if segment)
239 if not segments:
! 240 continue
241 if segments[0] == "auth":
! 242 segments = ("web",) + segments[1:]
243 _assign_path(overrides, segments, value)
244 return overrides
245 Lines 259-267 259 node = target.setdefault(key, {})
260 if isinstance(node, dict):
261 _merge_into(node, value)
262 else:
! 263 target[key] = deepcopy(value)
264 else:
265 target[key] = value
266 Lines 301-310 301 continue
302 cleaned[key] = cleaned_value
303 return cleaned
304 if isinstance(data, list):
! 305 cleaned_list = [_clean_data(item) for item in data]
! 306 return [item for item in cleaned_list if item is not None]
307 if data is None:
308 return None
309 return dataLines 329-340 329 base_path = Path(base_override).expanduser().resolve()
330 path = base_path / "config"
331 if _is_writable(path.parent):
332 return path
! 333 log.warning(
334 "'TIDY3D_BASE_DIR' is not writable; using temporary configuration directory instead."
335 )
! 336 return _temporary_config_dir()
337
338 canonical_dir = canonical_config_directory()
339 if _is_writable(canonical_dir.parent):
340 legacy_dir = legacy_config_directory()Lines 338-346 338 canonical_dir = canonical_config_directory()
339 if _is_writable(canonical_dir.parent):
340 legacy_dir = legacy_config_directory()
341 if legacy_dir.exists():
! 342 log.warning(
343 f"Using canonical configuration directory at '{canonical_dir}'. "
344 "Found legacy directory at '~/.tidy3d', which will be ignored. "
345 "Remove it manually or run 'tidy3d config migrate --delete-legacy' to clean up.",
346 log_once=True,Lines 348-360 348 return canonical_dir
349
350 legacy_dir = legacy_config_directory()
351 if legacy_dir.exists():
! 352 log.warning(
353 "Configuration found in legacy location '~/.tidy3d'. Consider running 'tidy3d config migrate'.",
354 log_once=True,
355 )
! 356 return legacy_dir
357
358 log.warning(f"Unable to write to '{canonical_dir}'; falling back to temporary directory.")
359 return _temporary_config_dir()Lines 361-369 361
362 def _xdg_config_home() -> Path:
363 xdg_home = os.getenv("XDG_CONFIG_HOME")
364 if xdg_home:
! 365 return Path(xdg_home).expanduser()
366 return Path.home() / ".config"
367
368
369 def _temporary_config_dir() -> Path:Lines 410-427 410 """
411
412 legacy_dir = legacy_config_directory()
413 if not legacy_dir.exists():
! 414 raise FileNotFoundError("Legacy configuration directory '~/.tidy3d' was not found.")
415
416 canonical_dir = canonical_config_directory()
417 if canonical_dir.resolve() == legacy_dir.resolve():
! 418 raise RuntimeError(
419 "Legacy and canonical configuration directories are the same path; nothing to migrate."
420 )
421
422 if canonical_dir.exists() and not overwrite:
! 423 raise FileExistsError(
424 f"Destination '{canonical_dir}' already exists. Pass overwrite=True to replace existing files."
425 )
426
427 canonical_dir.parent.mkdir(parents=True, exist_ok=True)Lines 433-440 433
434 finalize_legacy_migration(canonical_dir)
435
436 if remove_legacy:
! 437 shutil.rmtree(legacy_dir)
438
439 return canonical_dirtidy3d/_common/config/manager.pyLines 49-57 49
50 def __getattr__(self, name: str) -> Any:
51 model = self._manager._get_model(self._path)
52 if model is None:
! 53 raise AttributeError(f"Section '{self._path}' is not available")
54 return getattr(model, name)
55
56 def __setattr__(self, name: str, value: Any) -> None:
57 if name.startswith("_"):Lines 59-81 59 return
60 self._manager.update_section(self._path, **{name: value})
61
62 def __repr__(self) -> str:
! 63 model = self._manager._get_model(self._path)
! 64 return f"SectionAccessor({self._path}={model!r})"
65
66 def __rich__(self) -> Panel:
! 67 model = self._manager._get_model(self._path)
! 68 if model is None:
! 69 return Panel(Text(f"Section '{self._path}' is unavailable", style="red"))
! 70 data = _prepare_for_display(model.model_dump(exclude_unset=False))
! 71 return _build_section_panel(self._path, data)
72
73 def dict(self, *args: Any, **kwargs: Any) -> dict[str, Any]:
! 74 model = self._manager._get_model(self._path)
! 75 if model is None:
! 76 return {}
! 77 return model.model_dump(*args, **kwargs)
78
79 def __str__(self) -> str:
80 return self._manager.format_section(self._path)Lines 87-99 87 self._manager = manager
88
89 def __getattr__(self, plugin: str) -> SectionAccessor:
90 if plugin not in self._manager._plugin_models:
! 91 raise AttributeError(f"Plugin '{plugin}' is not registered")
92 return SectionAccessor(self._manager, f"plugins.{plugin}")
93
94 def list(self) -> Iterable[str]:
! 95 return sorted(self._manager._plugin_models.keys())
96
97
98 class ProfilesAccessor:
99 """Read-only profile helper."""Lines 98-112 98 class ProfilesAccessor:
99 """Read-only profile helper."""
100
101 def __init__(self, manager: ConfigManager):
! 102 self._manager = manager
103
104 def list(self) -> dict[str, list[str]]:
! 105 return self._manager.list_profiles()
106
107 def __getattr__(self, profile: str) -> dict[str, Any]:
! 108 return self._manager.preview_profile(profile)
109
110
111 class ConfigManager:
112 """High-level orchestrator for tidy3d configuration."""Lines 152-164 152 return PluginsAccessor(self)
153
154 @property
155 def profiles(self) -> ProfilesAccessor:
! 156 return ProfilesAccessor(self)
157
158 def update_section(self, name: str, **updates: Any) -> None:
159 if not updates:
! 160 return
161 segments = name.split(".")
162 overrides = self._runtime_overrides[self._profile]
163 previous = deepcopy(overrides)
164 node = overridesLines 176-187 176 self._apply_handlers(section=name)
177
178 def switch_profile(self, profile: str) -> None:
179 if not profile:
! 180 raise ValueError("Profile name cannot be empty")
181 normalized = normalize_profile_name(profile)
182 if not normalized:
! 183 raise ValueError("Profile name cannot be empty")
184 self._profile = normalized
185 self._reload()
186
187 # Notify users when switching to a non-default profileLines 280-288 280 """Restore previously overridden environment variables."""
281
282 for key, previous in self._web_env_previous.items():
283 if previous is None:
! 284 os.environ.pop(key, None)
285 else:
286 os.environ[key] = previous
287 self._web_env_previous.clear()Lines 286-312 286 os.environ[key] = previous
287 self._web_env_previous.clear()
288
289 def list_profiles(self) -> dict[str, list[str]]:
! 290 profiles_dir = self._loader.config_dir / "profiles"
! 291 user_profiles = []
! 292 if profiles_dir.exists():
! 293 for path in profiles_dir.glob("*.toml"):
! 294 user_profiles.append(path.stem)
! 295 built_in = sorted(name for name in BUILTIN_PROFILES.keys())
! 296 return {"built_in": built_in, "user": sorted(user_profiles)}
297
298 def preview_profile(self, profile: str) -> dict[str, Any]:
! 299 builtin = self._loader.get_builtin_profile(profile)
! 300 base = self._loader.load_base()
! 301 overrides = self._loader.load_user_profile(profile)
! 302 view = deep_merge(builtin, base, overrides)
! 303 return deepcopy(view)
304
305 def get_section(self, name: str) -> BaseModel:
306 model = self._get_model(name)
307 if model is None:
! 308 raise AttributeError(f"Section '{name}' is not available")
309 return model
310
311 def as_dict(self, include_env: bool = True) -> dict[str, Any]:
312 """Return the current configuration tree, including defaults for all sections."""Lines 318-326 318
319 def __rich__(self) -> Panel:
320 """Return a rich renderable representation of the full configuration."""
321
! 322 return _build_config_panel(
323 title=f"Config (profile='{self._profile}')",
324 data=_prepare_for_display(self.as_dict(include_env=True)),
325 )Lines 337-345 337 """Return a string representation for an individual section."""
338
339 model = self._get_model(name)
340 if model is None:
! 341 raise AttributeError(f"Section '{name}' is not available")
342 data = _prepare_for_display(model.model_dump(exclude_unset=False))
343 panel = _build_section_panel(name, data)
344 return _render_panel(panel)Lines 394-404 394 plugin_name = name.split(".", 1)[1]
395 plugin_data = _deep_get(self._effective_tree, ("plugins", plugin_name)) or {}
396 try:
397 new_plugins[plugin_name] = schema(**plugin_data)
! 398 except Exception as exc:
! 399 log.error(f"Failed to load configuration for plugin '{plugin_name}': {exc}")
! 400 errors.append((name, exc))
401 continue
402 if name == "plugins":
403 continue
404 section_data = self._effective_tree.get(name, {})Lines 429-441 429 if handler is None:
430 continue
431 model = self._get_model(target)
432 if model is None:
! 433 continue
434 try:
435 handler(model)
! 436 except Exception as exc:
! 437 log.error(f"Failed to apply configuration handler for '{target}': {exc}")
438
439 def _compose_without_env(self) -> dict[str, Any]:
440 runtime = self._runtime_overrides.get(self._profile, {})
441 return deep_merge(self._raw_tree, runtime)Lines 464-472 464 if name.startswith("plugins."):
465 plugin_name = name.split(".", 1)[1]
466 plugin_data = plugins_source.get(plugin_name, {})
467 if not isinstance(plugin_data, dict):
! 468 continue
469 persisted_plugin = _extract_persisted(schema, plugin_data)
470 if persisted_plugin:
471 plugin_filtered[plugin_name] = persisted_plugin
472 continueLines 472-480 472 continue
473
474 section_data = tree.get(name, {})
475 if not isinstance(section_data, dict):
! 476 continue
477 persisted_section = _extract_persisted(schema, section_data)
478 if persisted_section:
479 filtered[name] = persisted_sectionLines 485-493 485 def __getattr__(self, name: str) -> Any:
486 if name in self._section_models:
487 return SectionAccessor(self, name)
488 if name == "plugins":
! 489 return self.plugins
490 raise AttributeError(f"Config has no section '{name}'")
491
492 def __setattr__(self, name: str, value: Any) -> None:
493 if name.startswith("_"):Lines 492-507 492 def __setattr__(self, name: str, value: Any) -> None:
493 if name.startswith("_"):
494 object.__setattr__(self, name, value)
495 return
! 496 if name in self._section_models:
! 497 if isinstance(value, BaseModel):
! 498 payload = value.model_dump(exclude_unset=False)
499 else:
! 500 payload = value
! 501 self.update_section(name, **payload)
! 502 return
! 503 object.__setattr__(self, name, value)
504
505 def __str__(self) -> str:
506 return self.format()Lines 509-517 509 def _deep_get(tree: dict[str, Any], path: Iterable[str]) -> Optional[dict[str, Any]]:
510 node: Any = tree
511 for segment in path:
512 if not isinstance(node, dict):
! 513 return None
514 node = node.get(segment)
515 if node is None:
516 return None
517 return node if isinstance(node, dict) else NoneLines 520-528 520 def _resolve_model_type(annotation: Any) -> Optional[type[BaseModel]]:
521 """Return the first BaseModel subclass found in an annotation (if any)."""
522
523 if isinstance(annotation, type) and issubclass(annotation, BaseModel):
! 524 return annotation
525
526 origin = get_origin(annotation)
527 if origin is None:
528 return NoneLines 529-537 529
530 for arg in get_args(annotation):
531 nested = _resolve_model_type(arg)
532 if nested is not None:
! 533 return nested
534 return None
535
536
537 def _serialize_value(value: Any) -> Any:Lines 535-545 535
536
537 def _serialize_value(value: Any) -> Any:
538 if isinstance(value, BaseModel):
! 539 return value.model_dump(exclude_unset=False)
540 if hasattr(value, "get_secret_value"):
! 541 return value.get_secret_value()
542 return value
543
544
545 def _prepare_for_display(value: Any) -> Any:Lines 543-551 543
544
545 def _prepare_for_display(value: Any) -> Any:
546 if isinstance(value, BaseModel):
! 547 return {
548 k: _prepare_for_display(v) for k, v in value.model_dump(exclude_unset=False).items()
549 }
550 if isinstance(value, dict):
551 return {str(k): _prepare_for_display(v) for k, v in value.items()}Lines 551-566 551 return {str(k): _prepare_for_display(v) for k, v in value.items()}
552 if isinstance(value, (list, tuple, set)):
553 return [_prepare_for_display(v) for v in value]
554 if isinstance(value, Path):
! 555 return str(value)
556 if isinstance(value, Enum):
! 557 return value.value
558 if hasattr(value, "get_secret_value"):
! 559 displayed = getattr(value, "display", None)
! 560 if callable(displayed):
! 561 return displayed()
! 562 return str(value)
563 return value
564
565
566 def _build_config_panel(title: str, data: dict[str, Any]) -> Panel:Lines 569-577 569 for key in sorted(data.keys()):
570 branch = tree.add(Text(key, style="bold magenta"))
571 branch.add(Pretty(data[key], expand_all=True))
572 else:
! 573 tree.add(Text("<empty>", style="dim"))
574 return Panel(tree, border_style="cyan", padding=(0, 1))
575
576
577 def _build_section_panel(name: str, data: Any) -> Panel:Lines 590-598 590 def _model_dict(model: BaseModel) -> dict[str, Any]:
591 data = model.model_dump(exclude_unset=False)
592 for key, value in list(data.items()):
593 if hasattr(value, "get_secret_value"):
! 594 data[key] = value.get_secret_value()
595 return data
596
597
598 def _extract_persisted(schema: type[BaseModel], data: dict[str, Any]) -> dict[str, Any]:Lines 611-626 611 continue
612
613 nested_type = _resolve_model_type(annotation)
614 if nested_type is not None:
! 615 nested_source = value if isinstance(value, dict) else {}
! 616 nested_persisted = _extract_persisted(nested_type, nested_source)
! 617 if nested_persisted:
! 618 persisted[field_name] = nested_persisted
! 619 continue
620
621 if hasattr(value, "get_secret_value"):
! 622 persisted[field_name] = value.get_secret_value()
623 else:
624 persisted[field_name] = deepcopy(value)
625
626 return persistedtidy3d/_common/config/serializer.pyLines 36-48 36 descriptions[prefix] = description
37
38 nested_models: Iterable[type[BaseModel]] = _iter_model_types(field.annotation)
39 for model in nested_models:
! 40 nested_doc = (model.__doc__ or "").strip()
! 41 if nested_doc:
! 42 descriptions[prefix] = descriptions.get(prefix, nested_doc.splitlines()[0].strip())
! 43 for sub_name, sub_field in model.model_fields.items():
! 44 descriptions.update(_describe_field(sub_field, prefix=(*prefix, sub_name)))
45 return descriptions
46
47
48 def _iter_model_types(annotation: Any) -> Iterable[type[BaseModel]]:Lines 48-56 48 def _iter_model_types(annotation: Any) -> Iterable[type[BaseModel]]:
49 """Yield BaseModel subclasses referenced by a field annotation (if any)."""
50
51 if annotation is None:
! 52 return
53
54 stack = [annotation]
55 seen: set[type[BaseModel]] = set()Lines 56-67 56
57 while stack:
58 current = stack.pop()
59 if isinstance(current, type) and issubclass(current, BaseModel):
! 60 if current not in seen:
! 61 seen.add(current)
! 62 yield current
! 63 continue
64
65 origin = get_origin(current)
66 if origin is None:
67 continueLines 128-136 128 container.add(key, table)
129 return
130
131 if value is None:
! 132 return
133
134 existing_item = container.get(key)
135 new_item = tomlkit.item(value)
136 if isinstance(existing_item, Item):tidy3d/_common/log.pyLines 53-61 53
54 def _get_level_int(level: LogValue) -> int:
55 """Get the integer corresponding to the level string."""
56 if isinstance(level, int):
! 57 return level
58
59 if level not in _level_value:
60 # We don't want to import ConfigError to avoid a circular dependency
61 raise ValueError(Lines 90-99 90 # We want the calling site for exceptions.py
91 offset += 1
92 prefix, msg = self.log_level_format(level_name, message)
93 if self.prefix_every_line:
! 94 wrapped_text = Text(msg, style="default")
! 95 msgs = wrapped_text.wrap(console=console, width=console.width - len(prefix) - 2)
96 else:
97 msgs = [msg]
98 for msg in msgs:
99 console.log(Lines 217-225 217 # array field
218 new_loc = current_loc + list(field)
219 else:
220 # single field
! 221 new_loc = [*current_loc, field]
222
223 # process current level warnings
224 for level, msg, custom_loc in stack_item["messages"]:
225 if level == "WARNING":Lines 226-234 226 self._captured_warnings.append({"loc": new_loc + custom_loc, "msg": msg})
227
228 # initialize processing at children level
229 for child_stack in stack_item["children"].values():
! 230 self._parse_warning_capture(current_loc=new_loc, stack_item=child_stack)
231
232 else: # for root object
233 # process current level warnings
234 for level, msg, custom_loc in stack_item["messages"]:Lines 262-271 262 if len(args) > 0:
263 try:
264 composed_message = str(message) % args
265
! 266 except Exception as e:
! 267 composed_message = f"{message} % {args}\n{e}"
268 else:
269 composed_message = str(message)
270
271 # Capture all messages (even if suppressed later)Lines 299-311 299 self._log(_level_value["DEBUG"], "DEBUG", message, *args, log_once=log_once)
300
301 def support(self, message: str, *args: Any, log_once: bool = False) -> None:
302 """Log (message) % (args) at support level"""
! 303 self._log(_level_value["SUPPORT"], "SUPPORT", message, *args, log_once=log_once)
304
305 def user(self, message: str, *args: Any, log_once: bool = False) -> None:
306 """Log (message) % (args) at user level"""
! 307 self._log(_level_value["USER"], "USER", message, *args, log_once=log_once)
308
309 def info(self, message: str, *args: Any, log_once: bool = False) -> None:
310 """Log (message) % (args) at info level"""
311 self._log(_level_value["INFO"], "INFO", message, *args, log_once=log_once)Lines 369-377 369 stderr : bool
370 If False, logs are directed to stdout, otherwise to stderr.
371 """
372 if "console" in log.handlers:
! 373 previous_level = log.handlers["console"].level
374 else:
375 previous_level = DEFAULT_LEVEL
376 log.handlers["console"] = LogHandler(
377 Console(Lines 407-434 407 log_path : bool = False
408 Whether to log the path to the file that issued the message.
409 """
410 if filemode not in "wa":
! 411 raise ValueError("filemode must be either 'w' or 'a'")
412
413 # Close previous handler, if any
414 if "file" in log.handlers:
! 415 try:
! 416 log.handlers["file"].console.file.close()
! 417 except Exception: # TODO: catch specific exception
! 418 log.warning("Log file could not be closed")
419 finally:
! 420 del log.handlers["file"]
421
422 if str(fname) == "":
423 # Empty string can be passed to just stop previously opened file handler
! 424 return
425
426 try:
427 file = open(fname, filemode)
! 428 except Exception: # TODO: catch specific exception
! 429 log.error(f"File {fname} could not be opened")
! 430 return
431
432 log.handlers["file"] = LogHandler(
433 Console(file=file, force_jupyter=False, log_path=log_path), level
434 )Lines 443-451 443
444 def get_logging_console() -> Console:
445 """Get console from logging handlers."""
446 if "console" not in log.handlers:
! 447 set_logging_console()
448 return log.handlers["console"].console
449
450
451 class NoOpProgress:tidy3d/config/profiles.pyLines 2-11 2
3 # ruff: noqa: F401
4
5 # marked as migrated to _common
! 6 from __future__ import annotations
7
! 8 from tidy3d._common.config.profiles import (
9 BUILTIN_PROFILES,
10 )tidy3d/config/serializer.pyLines 2-12 2
3 # ruff: noqa: F401
4
5 # marked as migrated to _common
! 6 from __future__ import annotations
7
! 8 from tidy3d._common.config.serializer import (
9 Path,
10 _apply_value,
11 _describe_field,
12 _iter_model_types, |
marcorudolphflex
force-pushed
the
FXC-4683-move-config-to-new-common-submodule
branch
from
9c6c053 to
47cd85e
Compare
marcorudolphflex
requested review from
bzhangflex,
caseyflex,
dmarek-flex,
groberts-flex,
marc-flex,
tylerflex and
weiliangjin2021
as code owners
marcorudolphflex
changed the base branch from
develop
to
yaugenst-flex/pydantic-v2
marcorudolphflex
requested review from
mahlau-flex
and removed request for
bzhangflex,
caseyflex,
daquinteroflex,
dmarek-flex,
groberts-flex,
marc-flex,
tylerflex and
weiliangjin2021
marcorudolphflex
force-pushed
the
FXC-4683-move-config-to-new-common-submodule
branch
from
47cd85e to
ec50bca
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ] | ||
|
|
||
|
|
||
| def _create_manager() -> ConfigManager: |
There was a problem hiding this comment.
Missing __getattr__ breaks module-level attribute access
High Severity
The old tidy3d/config/__init__.py had a module-level __getattr__ function that delegated attribute access to the config wrapper, allowing patterns like tidy3d.config.logging_level or tidy3d.config.web. This __getattr__ was removed during the migration but not added to the new shim. Existing documentation (e.g., in tidy3d/__init__.py line 481 and dispersion_fitter.py line 166) tells users to use tidy3d.config.logging_level, which will now raise AttributeError. The shim needs to include def __getattr__(name): return getattr(_common_config, name) to maintain backward compatibility.
Contributor
Author
There was a problem hiding this comment.
Did a test, still works as tidy3d.__init__ does from .config import config and our config shim imports config from _common which supports __getattr__
marcorudolphflex
force-pushed
the
FXC-4683-move-config-to-new-common-submodule
branch
from
ec50bca to
80e03f3
Compare
|
|
||
| environment = Env | ||
|
|
||
| environment.set_current(self) |
There was a problem hiding this comment.
Missing null check may crash when Env uninitialized
Low Severity
The active() method imports Env from tidy3d._common.config and assigns it to environment, then calls environment.set_current(self). Since Env is lazily initialized and starts as None, this code path crashes with AttributeError if called before initialize_env() runs. The similar pattern in switch_profile() correctly guards against this with if _legacy_env is not None:, but active() lacks this null check. This affects the edge case where a LegacyEnvironmentConfig is created without an environment parameter and active() is called before proper initialization.
Contributor
Author
There was a problem hiding this comment.
Won't happen as we initialize on import. Or do you see a risk here?
mahlau-flex
force-pushed
the
yaugenst-flex/pydantic-v2
branch
5 times, most recently
from
3bf99ff to
7619bba
Compare
|
Too many files changed for review. |
Contributor
Author
|
included in #3138 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moved the
configmodule (exceptsections) and some related modules (_runtimeandlog) to the new_commonmodule._commonmodules in old module# marked as migrated to _commonto have a safe indicator when manipulating files with codetidy3d._common...now_commontidy3dimport in_commonHad to do this workaround in the old
tidy3d.config:__init___:with new
tidy3d.config:__init___being:Otherwise we run into this
Seems like the import dependencies are still quite chaotic here.
Not really happy with that, but we will deprecate the legacy
Envsoon anyways. Or do you see other solutions here?Greptile Summary
This PR refactors the
config,_runtime, andlogmodules by moving them fromtidy3dto a newtidy3d._commonsubmodule. The old module locations become compatibility shims that re-export all symbols from_common, maintaining backward compatibility.Key Changes:
tidy3d._commonmodule with migratedconfig,log, and_runtimemodules_commonmodules now usetidy3d._common.*paths to avoid circular dependencies_common(prevent imports from non-_commontidy3dmodules)Envinitialization by lazily creating it via__getattr__and callinginitialize_env()after sections are registered# marked as migrated to _commoncommentIssues Found:
ensure_common_imports.pyvs actualensure_imports_from_common.py) - will cause build failurereload_config()intidy3d._common.config.__init__.py:69will crash withAttributeErrorifEnvhasn't been initialized yet, since it's lazily initialized but code doesn't check forNoneArchitecture Notes:
The circular import workaround is acceptable for legacy code that's planned for deprecation in 2.12. The overall migration strategy is sound: move implementation to
_common, make old locations thin compatibility shims, and enforce import boundaries via CI.Confidence Score: 2/5
AttributeErrorinreload_config()reload_config()can crash withAttributeErrorwhenEnvisNone. The refactoring approach is sound, but these bugs must be fixed before merge..github/workflows/tidy3d-python-client-tests.yml(wrong script name) andtidy3d/_common/config/__init__.py(reload_config bug)Important Files Changed
ensure_common_imports.pyinstead ofensure_imports_from_common.py)_commonimports in_commonmodules, well-structured with proper error reporting_commonconfig module with lazyEnvinitialization, butreload_config()has bug whenEnvis Noneinitialize_env()to work around circular dependencySequence Diagram
sequenceDiagram participant User participant tidy3d.config participant tidy3d._common.config participant sections participant Env User->>tidy3d.config: import tidy3d.config tidy3d.config->>tidy3d._common.config: import _common_config tidy3d._common.config->>tidy3d._common.config: Create _base_manager Note over tidy3d._common.config: Env = None (lazy init) tidy3d.config->>sections: from tidy3d.config import sections sections->>sections: Register sections with manager tidy3d.config->>tidy3d._common.config: initialize_env() tidy3d._common.config->>Env: Env = LegacyEnvironment(_base_manager) Note over Env: Now sections are available tidy3d.config->>tidy3d._common.config: Re-export all symbols User->>tidy3d.config: Access config.Env tidy3d.config->>tidy3d.config: __getattr__("Env") tidy3d.config->>tidy3d._common.config: initialize_env() (if needed) tidy3d.config-->>User: Return EnvNote
Moves
config,log, and_runtimeimplementations into a newtidy3d._commonpackage and converts old modules to thin compatibility shims that re-export from_common._commonsubmodules:config(manager, loader, legacy, profiles, registry, serializer),log,_runtime, andcompat; updates internal imports totidy3d._common.*tidy3d.config/*,tidy3d.log,tidy3d._runtime, and related files as shims with explicit "marked as migrated to _common" comments; adds lazyEnvinitialization to avoid import cyclesensure-common-importsrunningscripts/ensure_imports_from_common.pyand gates workflow on its result_common, update a type import inplugins/design/design.pyWritten by Cursor Bugbot for commit 80e03f3. This will update automatically on new commits. Configure here.