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.

Sign up for GitHub

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

Jump to bottom

Update the types of dataclass attributes according to usage #163

Merged
merged 2 commits into from
Nov 1, 2020
Merged

Update the types of dataclass attributes according to usage #163

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a050e9d
Update the types of dataclass attributes according to usage
kvid Sep 1, 2020
c04804c
Add type aliases that reflect their semantics
kvid Sep 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 62 additions & 42 deletions src/wireviz/DataClasses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,46 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-

from typing import Optional, List, Any, Union
from typing import Optional, List, Tuple, Union
from dataclasses import dataclass, field, InitVar
from pathlib import Path
from wireviz.wv_helper import int2tuple, aspect_ratio
from wireviz import wv_colors


# Each type alias have their legal values described in comments - validation might be implemented in the future
PlainText = str # Text not containing HTML tags nor newlines
Hypertext = str # Text possibly including HTML hyperlinks that are removed in all outputs except HTML output
MultilineHypertext = str # Hypertext possibly also including newlines to break lines in diagram output
Designator = PlainText # Case insensitive unique name of connector or cable

# Literal type aliases below are commented to avoid requiring python 3.8
ConnectorMultiplier = str # = Literal['pincount', 'populated']
CableMultiplier = str # = Literal['wirecount', 'terminations', 'length', 'total_length']
ConnectorMultiplier = PlainText # = Literal['pincount', 'populated']
CableMultiplier = PlainText # = Literal['wirecount', 'terminations', 'length', 'total_length']
ImageScale = PlainText # = Literal['false', 'true', 'width', 'height', 'both']
Color = PlainText # Two-letter color name = Literal[wv_colors._color_hex.keys()]
ColorScheme = PlainText # Color scheme name = Literal[wv_colors.COLOR_CODES.keys()]

# Type combinations
Colors = PlainText # One or more two-letter color names (Color) concatenated into one string
Pin = Union[int, PlainText] # Pin identifier
Wire = Union[int, PlainText] # Wire number or Literal['s'] for shield
NoneOrMorePins = Union[Pin, Tuple[Pin, ...], None] # None, one, or a tuple of pins
OneOrMoreWires = Union[Wire, Tuple[Wire, ...]] # One or a tuple of wires


@dataclass
class Image:
gv_dir: InitVar[Path] # Directory of .gv file injected as context during parsing
# Attributes of the image object <img>:
src: str
scale: Optional[str] = None # false | true | width | height | both
scale: Optional[ImageScale] = None
# Attributes of the image cell <td> containing the image:
width: Optional[int] = None
height: Optional[int] = None
fixedsize: Optional[bool] = None
# Contents of the text cell <td> just below the image cell:
caption: Optional[str] = None
caption: Optional[MultilineHypertext] = None
# See also HTML doc at https://graphviz.org/doc/info/shapes.html#html

def __post_init__(self, gv_dir):
Expand All @@ -47,13 +64,14 @@ def __post_init__(self, gv_dir):
if self.width:
self.height = self.width / aspect_ratio(gv_dir.joinpath(self.src))


@dataclass
class AdditionalComponent:
type: str
subtype: Optional[str] = None
manufacturer: Optional[str] = None
mpn: Optional[str] = None
pn: Optional[str] = None
type: MultilineHypertext
subtype: Optional[MultilineHypertext] = None
manufacturer: Optional[MultilineHypertext] = None
mpn: Optional[MultilineHypertext] = None
pn: Optional[Hypertext] = None
Comment on lines +72 to +74
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe the answer to my question is buried in the discussion on #115, but:

Why are manufacturer and mpn multiline-capable, and pn is not?

Personally, I'm not sure why any of them would need to be multiline, but at least it should be consistend, since all of them are passed through the html_line_breaks() function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe the answer to my question is buried in the discussion on #115, but:

This feature is much older that #115.

Why are manufacturer and mpn multiline-capable, and pn is not?

Blame tells me the change was committed by you in 102c7d6 as suggested by me in #136 (comment): In case a user need a long manufacturer info, I suggest supporting line breaks to avoid a very wide node. And since both manufacturer and mpn was placed in the same table cell, I suggested calling html_line_breaks on the whole cell contents.

Personally, I'm not sure why any of them would need to be multiline, but at least it should be consistend, since all of them are passed through the html_line_breaks() function.

Currently, (since the commit mentioned above that is included in v0.2) manufacturer and mpn are passed through the html_line_breaks() function, but not pn, and that's why I used different type hints to reflect how it is currently implemented.

I understand your argument about consistency, but then we also need to change the implementation. I have an idea about moving html_line_breaks() in #168 that will solve your consistency request, but it's still WIP.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for digging through history for me :)

OK, then let's keep it as-is to reflect the current implementation, and I'll wait for #168.

qty: float = 1
unit: Optional[str] = None
qty_multiplier: Union[ConnectorMultiplier, CableMultiplier, None] = None
Expand All @@ -65,29 +83,29 @@ def description(self) -> str:

@dataclass
class Connector:
name: str
manufacturer: Optional[str] = None
mpn: Optional[str] = None
pn: Optional[str] = None
name: Designator
manufacturer: Optional[MultilineHypertext] = None
mpn: Optional[MultilineHypertext] = None
pn: Optional[Hypertext] = None
style: Optional[str] = None
category: Optional[str] = None
type: Optional[str] = None
subtype: Optional[str] = None
type: Optional[MultilineHypertext] = None
subtype: Optional[MultilineHypertext] = None
pincount: Optional[int] = None
image: Optional[Image] = None
notes: Optional[str] = None
pinlabels: List[Any] = field(default_factory=list)
pins: List[Any] = field(default_factory=list)
color: Optional[str] = None
show_name: bool = None
show_pincount: bool = None
notes: Optional[MultilineHypertext] = None
pinlabels: List[Pin] = field(default_factory=list)
pins: List[Pin] = field(default_factory=list)
color: Optional[Color] = None
show_name: Optional[bool] = None
show_pincount: Optional[bool] = None
hide_disconnected_pins: bool = False
autogenerate: bool = False
loops: List[Any] = field(default_factory=list)
loops: List[List[Pin]] = field(default_factory=list)
ignore_in_bom: bool = False
additional_components: List[AdditionalComponent] = field(default_factory=list)

def __post_init__(self):
def __post_init__(self) -> None:

if isinstance(self.image, dict):
self.image = Image(**self.image)
Expand Down Expand Up @@ -139,7 +157,7 @@ def __post_init__(self):
if isinstance(item, dict):
self.additional_components[i] = AdditionalComponent(**item)

def activate_pin(self, pin):
def activate_pin(self, pin: Pin) -> None:
self.visible_pins[pin] = True

def get_qty_multiplier(self, qty_multiplier: Optional[ConnectorMultiplier]) -> int:
Expand All @@ -155,29 +173,29 @@ def get_qty_multiplier(self, qty_multiplier: Optional[ConnectorMultiplier]) -> i

@dataclass
class Cable:
name: str
manufacturer: Optional[Union[str, List[str]]] = None
mpn: Optional[Union[str, List[str]]] = None
pn: Optional[Union[str, List[str]]] = None
name: Designator
manufacturer: Union[MultilineHypertext, List[MultilineHypertext], None] = None
mpn: Union[MultilineHypertext, List[MultilineHypertext], None] = None
pn: Union[Hypertext, List[Hypertext], None] = None
category: Optional[str] = None
type: Optional[str] = None
type: Optional[MultilineHypertext] = None
gauge: Optional[float] = None
gauge_unit: Optional[str] = None
show_equiv: bool = False
length: float = 0
kvid marked this conversation as resolved.
Show resolved Hide resolved
color: Optional[str] = None
color: Optional[Color] = None
wirecount: Optional[int] = None
shield: bool = False
shield: Union[bool, Color] = False
image: Optional[Image] = None
notes: Optional[str] = None
colors: List[Any] = field(default_factory=list)
color_code: Optional[str] = None
notes: Optional[MultilineHypertext] = None
colors: List[Colors] = field(default_factory=list)
color_code: Optional[ColorScheme] = None
show_name: bool = True
show_wirecount: bool = True
ignore_in_bom: bool = False
additional_components: List[AdditionalComponent] = field(default_factory=list)

def __post_init__(self):
def __post_init__(self) -> None:

if isinstance(self.image, dict):
self.image = Image(**self.image)
Expand Down Expand Up @@ -237,7 +255,9 @@ def __post_init__(self):
if isinstance(item, dict):
self.additional_components[i] = AdditionalComponent(**item)

def connect(self, from_name, from_pin, via_pin, to_name, to_pin):
# The *_pin arguments accept a tuple, but it seems not in use with the current code.
def connect(self, from_name: Optional[Designator], from_pin: NoneOrMorePins, via_pin: OneOrMoreWires,
to_name: Optional[Designator], to_pin: NoneOrMorePins) -> None:
from_pin = int2tuple(from_pin)
via_pin = int2tuple(via_pin)
to_pin = int2tuple(to_pin)
Expand All @@ -264,8 +284,8 @@ def get_qty_multiplier(self, qty_multiplier: Optional[CableMultiplier]) -> float

@dataclass
class Connection:
from_name: Any
from_port: Any
via_port: Any
to_name: Any
to_port: Any
from_name: Optional[Designator]
from_port: Optional[Pin]
via_port: Wire
to_name: Optional[Designator]
to_port: Optional[Pin]