-
Notifications
You must be signed in to change notification settings - Fork 15
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
Adding support for fields that are excluded from init and exclusively in the init #67
base: main
Are you sure you want to change the base?
Conversation
def _is_classvar(a_type): | ||
return a_type is ClassVar or ( | ||
type(a_type) is _ClassVarType and a_type.__origin__ is ClassVar | ||
) | ||
|
||
|
||
def _is_initvar(a_type): | ||
return a_type is InitVar or type(a_type) is InitVar |
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.
These are copied and modified from the dataclasses
module.
if dc_fields is None: | ||
return fields(dataclass) |
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.
Using __dataclass_fields__
seemed to be the only way to get the InitVar
fields.
I was not sure what to do if it was not found. I decided to fall back to dataclasses.fields
because that currently raise an exception if that attribute is not found.
What are the use cases for supporting |
Admittedly when I first started adding support for After thinking about it a bit I have two theoretical use cases. These use cases are contrived but I don't think they are unreasonable. Combining two or more flags into a single enumRather than have someone specify a enum value by name or value from the command line. There can be a few flag arguments that are used to produce a single value. Out of the two use causes I came up with, this is the one I can't think of an way to do the equivalent functionality with with current implementation. Example: class SupportedBuilds(Enum):
WINDOWS_X86 = auto()
WINDOWS_X64 = auto()
LINUX_X32 = auto()
LINUX_X64 = auto()
LINUX_ARM64 = auto()
MACOS_ARM64 = auto()
@staticmethod
def from_flags( os: str, is_32Bit: bool, is_arm_arch: bool) -> 'SupportedBuilds':
... # raise an error if flag combination is not supported
@dataclass
class Options:
os: InitVar[Literal["windows", "linux", "mac"]] = "windows"
is_32Bit: InitVar[bool] = field(default=False, metadata={"args": ["--32bit"]})
is_arm_arch: InitVar[bool] = field(default=False, metadata={"args": ["--arm"]})
#
selected_build: SupportedBuilds = field(init=False)
def __post_init__(self, os: str, is_32Bit: bool, is_arm_arch: bool):
self.selected_build = SupportedBuilds.from_flags(os, is_32Bit, is_arm_arch) Generating multiple values from a single complex inputWith this use case there is a single command line parameter that get parsed into multiple parameters that the application can use. Example: def parse_timerange(time_range: str) -> tuple[datetime, datetime]:
# last * (days | hours | ...)
# from * to *
return ...
@dataclass
class Parameters:
# command line arg
time_range: InitVar[str] = "last 5 days"
# exposed parameter
start_date: datetime = field(init=False)
end_date: datetime = field(init=False)
def __post_init__(self, time_range: str):
#
self.start_date, self.end_date = parse_timerange(time_range) To be fair, this is an alternative approach that produces the same result. Alternative Example: class TimeRange(NamedTuple):
start: datetime
end: datetime
def parse_timerange2(time_range: str = "last 5 days") -> TimeRange:
# last * (days | hours | ...)
# from * to *
return TimeRange(datetime.min, datetime.now())
@dataclass
class Parameters2:
time_range: TimeRange = field(default_factory=parse_timerange2, metadata = {"type": parse_timerange2}) I have put more thought into this reply than into the implementation. 🫤 I started this just because I was looking into ways my change in pr 66 could break things.. |
d0b8030
to
9a34a17
Compare
9a34a17
to
3c76f8b
Compare
It was a lot more involved to get support for fields that are exclusively part the initialization.
If you feel that supporting this is not worth the complexity, I am okay with that.