-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pydantic v2 #103
Pydantic v2 #103
Conversation
|
||
def check_match_gpu(v: Optional[int], values: dict) -> Optional[int]: | ||
if v is not None and v > 0 and values.get("gpu") == "": | ||
def check_match_gpu(v: Optional[int], info: FieldValidationInfo) -> Optional[int]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validator function signature changed
return 0 # GPU explicitly disabled | ||
return v | ||
|
||
|
||
# models | ||
|
||
|
||
class PartitionResources(BaseModel, allow_mutation=False, extra=Extra.allow): | ||
class PartitionResources(BaseModel, frozen=True, extra="allow"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config param names changed and use of Extra....
is deprecated
@@ -44,12 +41,10 @@ class PartitionResources(BaseModel, allow_mutation=False, extra=Extra.allow): | |||
max_runtime: PositiveInt | |||
|
|||
# validators | |||
_check_match_gpu = validator("max_ngpus", allow_reuse=True)(check_match_gpu) | |||
_check_match_gpu = field_validator("max_ngpus")(check_match_gpu) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name change and now useless allow_reuse
param
"""Information about partition description and available environments""" | ||
|
||
architecture = "" | ||
description = "" | ||
architecture: str = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Types looks to be needed for all field even with a default value
"""Complete information about a partition: config and resources""" | ||
|
||
pass | ||
model_config = ConfigDict(frozen=True, extra="allow") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Former way of setting this no longer work with multiple inheritance
def check_is_strictly_positive_or_none(cls, v: Optional[int]) -> Optional[int]: | ||
if v is not None and v <= 0: | ||
raise ValueError("Value must be strictly positive") | ||
return v | ||
|
||
|
||
class PartitionsTrait(BaseModel, allow_mutation=False, extra=Extra.forbid): | ||
class PartitionsTrait(RootModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use __root__
replaced with inheritance from RootModel
|
||
def dict(self, *args, **kwargs): | ||
return {k: v.dict(*args, **kwargs) for k, v in self.__root__.items()} | ||
def model_dump(self, *args, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dict()
renamed model_dump()
@@ -169,9 +169,9 @@ def parse_formdata(cls, formdata: dict[str, list[str]]) -> UserOptions: | |||
fields["output"] = ( | |||
"slurm-%j.out" if fields.get("output", "false") == "true" else "/dev/null" | |||
) | |||
return cls.parse_obj(fields) | |||
return cls.model_validate(fields) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parse_ob()
renamed to model_validate()
def check_memory(cls, v: str) -> str: | ||
if v and cls._MEM_REGEXP.match(v) is None: | ||
if v and _MEM_REGEXP.match(v) is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was trouble with access to class attribute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comprehensive PR ❤️
This PR updates the code to run with pydantic v2 which was recently released and breaks a few APIs.
closes #102