protobuf_to_pydantic icon indicating copy to clipboard operation
protobuf_to_pydantic copied to clipboard

The generation logic for import statements has issues in some cases

Open kasegao opened this issue 1 year ago • 2 comments

Describe the bug

The _add_other_module_pkg function in protobuf_to_pydantic/plugin/field_desc_proto_to_code.py has issues in some cases. Specifically, in cases where the top-level paths are different and the intermediate paths match, the generated relative import paths are not appropriate.

To Reproduce

Consider the following directory structure. Both files share the intermediate path v1.

.
├─ app/v1/app.proto
└─ shared/v1/types.proto

app/v1/app.proto

syntax = "proto3";

package user.v1;

import "shared/v1/types.proto";

service AppService {
  rpc GetMe(GetMeRequest) returns (GetMeResponse);
}

message GetMeRequest {}
message GetMeResponse {
  shared.v1.UserType user_type = 1;
  string name = 2;
  int32 age = 3;
}

shared/v1/types.proto

syntax = "proto3";

package shared.v1;

enum UserType {
  USER_TYPE_UNSPECIFIED = 0;
  USER_TYPE_NORMAL = 1;
  USER_TYPE_GUEST = 2;
}

When generating Pydantic code for these files, the following files are created:

app/v1/app_p2p.py

from .types_p2p import UserType
from google.protobuf.message import Message  # type: ignore
from pydantic import BaseModel
from pydantic import Field


class GetMeRequest(BaseModel):    pass

class GetMeResponse(BaseModel):
    user_type: UserType = Field(default=0)
    name: str = Field(default="")
    age: int = Field(default=0)

shared/v1/types_p2p.py

from enum import IntEnum
from pydantic import BaseModel

class UserType(IntEnum):
    USER_TYPE_UNSPECIFIED = 0
    USER_TYPE_NORMAL = 1
    USER_TYPE_GUEST = 2

However, the import statement in the first line of app/v1/app_p2p.py fails to resolve the relative path correctly.

from .types_p2p import UserType

Expected behavior

~I suggest specifying the path from the project root instead of using a relative path. In that case, the previously mentioned import statement should preferably be~

from shared.v1.types_p2p import UserType

Expectied import path:

from ...shared.v1.types_p2p import UserType

Desktop (please complete the following information):

  • OS: macOS Sonoma 14.5
  • Version MacBook Pro M3 Max (2023)

kasegao avatar Aug 24 '24 10:08 kasegao

I'm sorry, but the expected behavior I originally wrote was inappropriate, so I updated it.

kasegao avatar Sep 01 '24 22:09 kasegao

Thank you very much for your feedback. There is no problem with this code in the current example proto. Can you add the corresponding proto to the example in the project and execute it? It would be great if you could add corresponding test cases @kasegao

so1n avatar Sep 09 '24 16:09 so1n

@so1n Hi. I do face a similar bug for relative import. Found out there is a bug in the _add_other_module_pkg function. It should use fd_path_list instead of message_path_list to decide how many levels should relative import have.

To Reproduce

Consider the following directory structure.

.
├─ testcase/app.proto
└─ testcase/enums/category.proto

testcase/app.proto

syntax = "proto3";

package models.testcase;

import "testcase/enums/category.proto";

message Application {
    string id = 1;
    string name = 2;
    string owner = 3;
    enums.AppCategory category = 4;
}

testcase/enums/category.proto

syntax = "proto3";

package models.testcase.enums;

enum AppCategory {
    AC_INVALID = 0;
    AC_NONE = 1;
    AC_WEB = 2;
    AC_DESKTOP = 3;
    AC_MOBILE = 4;
}

The corresponding code generated:

models/testcase/app_p2p.py

# This is an automatically generated file, please do not change
# gen by protobuf_to_pydantic[v0.3.0.2](https://github.com/so1n/protobuf_to_pydantic)
# Protobuf Version: 5.28.3 
# Pydantic Version: 2.10.1 
from ..enums.category_p2p import AppCategory
from google.protobuf.message import Message  # type: ignore
from pydantic import BaseModel
from pydantic import Field


class Application(BaseModel):
    id: str = Field(default="")
    name: str = Field(default="")
    owner: str = Field(default="")
    category: AppCategory = Field(default=0)

models/testcase/enums/category_p2p.py

# This is an automatically generated file, please do not change
# gen by protobuf_to_pydantic[v0.3.0.2](https://github.com/so1n/protobuf_to_pydantic)
# Protobuf Version: 5.28.3 
# Pydantic Version: 2.10.1 
from enum import IntEnum
from pydantic import BaseModel

class AppCategory(IntEnum):
    AC_INVALID = 0
    AC_NONE = 1
    AC_WEB = 2
    AC_DESKTOP = 3
    AC_MOBILE = 4

As you can see in the models/testcase/app_p2p.py file, the relative import path is wrong.

The expected import path is:

from .enums.category_p2p import AppCategory

Environment: Distributor ID: Ubuntu Description: Ubuntu 22.04.5 LTS Release: 22.04 Codename: jammy

I have created a PR to fix this

Alfredx avatar Dec 04 '24 08:12 Alfredx