Skip to content

Conversation

@mahlau-flex
Copy link
Contributor

@mahlau-flex mahlau-flex commented Oct 1, 2025

The original purpose of this PR was to update the web submodule as an isolated component to pydantic v2, since updating the core components (mainly Tidy3DBaseModel) would require a lot of effort.

However, gradual adaptation of pydantic V2 in the web submodule is more difficult than anticipated. The Tidy3DBaseModel (or subclasses thereof) are also used a lot in this submodule, making gradual adaptation almost impossible.

In this PR, I have adapted two isolated components to pydantic v2, since these components. This was possible since these components are not used within the tidy3D repo itself. I would need some help estimating if these changes could affect code outside of this repo.

It is also debatable if it even makes sense to only udpate few isolated components here, since they would need to be isolated until the rest of the repo is also updated to v2 (mixing of v1 and v2 immediately results in an exception). This could lead to problems in development later.

Greptile Overview

Updated On: 2025-10-01 12:11:50 UTC

Summary

This PR performs a partial migration of two isolated components in the web submodule from Pydantic v1 to v2. The changes update `tidy3d/web/core/s3utils.py` and `tidy3d/web/api/material_fitter.py` by switching imports from `pydantic.v1` to `pydantic` and updating deprecated method calls to their v2 equivalents.

In s3utils.py, the parse_obj() method is replaced with model_validate() for the _S3STSToken class. In material_fitter.py, the .json() method is updated to .model_dump_json() for JSON serialization. These components were selected because they use basic Pydantic functionality without depending on the core Tidy3DBaseModel classes that are extensively used throughout the repository.

The migration approach is intentionally limited in scope - the developer acknowledges that a full Pydantic v2 migration would be complex due to widespread use of Tidy3DBaseModel throughout the codebase. By updating only isolated components, this PR serves as a proof-of-concept for Pydantic v2 compatibility while avoiding the extensive refactoring that would be required for a complete migration.

PR Description Notes:

  • There's a grammatical error in the description: "since these components" appears to be an incomplete sentence
  • The developer expresses uncertainty about whether partial migration makes sense, given that mixing Pydantic v1 and v2 can cause exceptions
Changed Files
Filename Score Overview
tidy3d/web/core/s3utils.py 4/5 Updates import from pydantic.v1 to pydantic and replaces deprecated parse_obj() with model_validate()
tidy3d/web/api/material_fitter.py 3/5 Migrates to Pydantic v2 imports and updates JSON serialization, but misses updating .dict() to .model_dump()

Confidence score: 3/5

  • This PR introduces moderate risk due to incomplete migration and potential compatibility issues between Pydantic v1 and v2 components
  • Score reflects concerns about partial migration strategy and one missed method update that could cause runtime errors
  • Pay close attention to tidy3d/web/api/material_fitter.py which still uses the deprecated .dict() method on line 101

Sequence Diagram

sequenceDiagram
    participant User
    participant MaterialFitterTask
    participant FitterOptions
    participant DispersionFitter
    participant HTTP_API
    participant S3_Service
    participant TempFile
    
    User->>MaterialFitterTask: "submit(fitter, options)"
    MaterialFitterTask->>DispersionFitter: "validate fitter type"
    MaterialFitterTask->>FitterOptions: "validate options type"
    MaterialFitterTask->>DispersionFitter: "extract wvl_um, n_data, k_data"
    MaterialFitterTask->>TempFile: "create temporary CSV file"
    MaterialFitterTask->>TempFile: "save data with np.savetxt"
    MaterialFitterTask->>HTTP_API: "get signed URL for upload"
    MaterialFitterTask->>HTTP_API: "PUT request to upload file"
    MaterialFitterTask->>FitterOptions: "model_dump_json(exclude_none=True)"
    MaterialFitterTask->>HTTP_API: "POST fitter/fit request"
    HTTP_API-->>MaterialFitterTask: "return task response"
    
    User->>MaterialFitterTask: "sync_status()"
    MaterialFitterTask->>HTTP_API: "GET fitter/{id} status"
    HTTP_API-->>MaterialFitterTask: "update status"
    
    User->>MaterialFitterTask: "save_to_library(name)"
    MaterialFitterTask->>HTTP_API: "POST fitter/save request"
    HTTP_API-->>MaterialFitterTask: "return success status"
    
    User->>S3_Service: "upload_file(resource_id, path, filename)"
    S3_Service->>HTTP_API: "get_s3_sts_token()"
    HTTP_API-->>S3_Service: "return S3 token"
    S3_Service->>S3_Service: "validate token expiration"
    S3_Service->>TempFile: "open file for reading"
    S3_Service->>S3_Service: "upload_fileobj to S3"
    
    User->>S3_Service: "download_file(resource_id, filename)"
    S3_Service->>HTTP_API: "get_s3_sts_token()"
    S3_Service->>S3_Service: "head_object for metadata"
    S3_Service->>TempFile: "create temporary download file"
    S3_Service->>S3_Service: "download_file from S3"
    S3_Service->>TempFile: "rename temp file to final path"
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@mahlau-flex mahlau-flex force-pushed the pydantic_migration_stubs branch from 131e78c to 54e85ff Compare October 1, 2025 13:22
@mahlau-flex mahlau-flex changed the title 🚀 REFC: Updating web submodule to pydantic v2 REFC: Updating web submodule to pydantic v2 Oct 1, 2025
@yaugenst-flex yaugenst-flex force-pushed the pydantic_migration_stubs branch from 54e85ff to 09f8783 Compare October 1, 2025 13:24
Copy link
Collaborator

@yaugenst-flex yaugenst-flex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks @mahlau-flex!

@yaugenst-flex yaugenst-flex added this pull request to the merge queue Oct 1, 2025
@daquinteroflex daquinteroflex removed this pull request from the merge queue due to a manual request Oct 1, 2025
@daquinteroflex daquinteroflex added this pull request to the merge queue Oct 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 1, 2025
@yaugenst-flex yaugenst-flex force-pushed the pydantic_migration_stubs branch from 09f8783 to 28b74d9 Compare October 1, 2025 16:16
@yaugenst-flex yaugenst-flex added this pull request to the merge queue Oct 1, 2025
Merged via the queue into flexcompute:develop with commit 67edb48 Oct 1, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants