Skip to content
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

[internal] Using the Optional[] keyword in dataclasses #156

Closed
formatc1702 opened this issue Aug 16, 2020 · 4 comments
Closed

[internal] Using the Optional[] keyword in dataclasses #156

formatc1702 opened this issue Aug 16, 2020 · 4 comments

Comments

@formatc1702
Copy link
Collaborator

Currently, some fields of the classes in DataClasses.py use the Optional[] keyword, and some don't.

class Connector:
    ...
    color: Optional[str] = None
    show_name: bool = None

I am unsure if there is a difference in behavior between both; or if the = None is needed at all.
Boolean values should probably have a default value of False anyway, instead of None.

Perhaps someone can chime in and suggest which variant is recommended; then I can create a PR to make the syntax more consistent. Thanks!

@kvid
Copy link
Collaborator

kvid commented Aug 16, 2020

...; or if the = None is needed at all.
Boolean values should probably have a default value of False anyway, instead of None.

When = None is used, then it's possible to differ between setting the attribute to false and not specifying it in the YAML file. For some attributes this difference is important, e.g. when a default value depends on other factors, like for show_pincount:

        if self.show_pincount is None:
            self.show_pincount = self.style != 'simple' # hide pincount for simple (1 pin) connectors by default

@kvid
Copy link
Collaborator

kvid commented Aug 20, 2020

Currently, some fields of the classes in DataClasses.py use the Optional[] keyword, and some don't.

class Connector:
    ...
    color: Optional[str] = None
    show_name: bool = None

I am unsure if there is a difference in behavior between both; or if the = None is needed at all.

There are two different concepts:

  • A field can have a default value. The generated __init__() method will then get an optional argument for this field, and no exception is raised when a value for this field is missing in the YAML input.
  • A field can have an optional type, i.e. None is allowed as a value in addition to the annotated type.

Optional[X] is used in the latter case when the type is optional, independently of whether it has a default value or not, but when a default value is None, then the type also is optional.

Excerpt from the doc below:

Optional type.

Optional[X] is equivalent to Union[X, None].

Note that this is not the same concept as an optional argument, which is one that has a default. An optional argument with a default does not require the Optional qualifier on its type annotation just because it is optional. For example:

def foo(arg: int = 0) -> None:
    ...

On the other hand, if an explicit value of None is allowed, the use of Optional is appropriate, whether the argument is optional or not. For example:

def foo(arg: Optional[int] = None) -> None:
    ...

@kvid
Copy link
Collaborator

kvid commented Sep 5, 2020

Should we start using type annotations in other code than DataClasses.py as well?

@formatc1702 formatc1702 added this to the v0.3 milestone Oct 17, 2020
kvid added a commit to kvid/WireViz that referenced this issue Oct 27, 2020
@formatc1702 formatc1702 removed the help wanted Extra attention is needed label Oct 30, 2020
formatc1702 added a commit that referenced this issue Nov 1, 2020
@formatc1702
Copy link
Collaborator Author

I guess I can leave this issue open even after closing #163 in response to @kvid's last comment:

Should we start using type annotations in other code than DataClasses.py as well?

because yes, I believe it's worth it including more type hints.

@formatc1702 formatc1702 removed this from the v0.3 milestone Oct 7, 2021
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

No branches or pull requests

2 participants