Implement mates between connectors, better autogeneration #186

17o2 · 2020-10-22T16:25:54Z

This is a sandbox to try an implementation of #134 and #184 (with possible effects for #142 and #185).
Since both involve changing the WireViz syntax, I figured it might make sense to tackle both at the same time, to make sure there are no conflicts between the two syntax additions.

Discussion regarding the new syntax for mating connectors happens in [feature] Connector-Connector & Connector-Ferrule mating #134.
Discussion regarding the new syntax for auto-generated components happens in [feature] Improved auto-generated connectors and cables #184.

This PR will try to reflect the results in those discussions.

The implementation will work towards being able to parse the included test/test1.yml and produce the expected result. This test file will try to reflect and test any related new features suggested in the discussions.

No detailed code review is necessary at the moment, consider this a work in progress;

17o2 · 2020-10-24T13:30:46Z

The current state is able to parse all current examples/tutorials/demos (only minor changes to the source file necessary), as well as all test files, inspired by the discussions in the issues mentioned above. See tests/ directory for sources.

Main open TODO is re-implementing the check for alternating connectors and cables inside each connection set.

See renderings

Test 1

Test 2

Test 3

Test 4

Test 9

17o2 · 2021-03-20T11:01:44Z

Even though I have not worked on this feature for a while, I have been using it in practice and it has proven quite robust.

I would like to request a review of the code, and I am open for ideas on how to solve the open TODO...

@kvid how do you feel about this?

17o2 · 2021-09-23T15:10:40Z

The branch has been rebased onto dev
(using squash to avoid needing to solve merge conflicts on intermediate commits;
a backup branch exists for now to preserve the original history)
All checks are passing and no merge conflicts await
Output for all examples and tutorials looks good
There are some minor TODOs which I would like to address in separate PRs to avoid delays (the branch has been essentially unchanged for almost a year)
The code has seen months of real life use (by me) without problems
It works as well as / better than the old code

I realize this is a major refactoring of the WireViz core. Nevertheless, it would be great to merge this to add some valuable new functionality to the project.

I am toying with the idea of trying out the "show" type PR merge as proposed by Martin Fowler to speed things up. The linked article is based on having a proper CI system; the bullet points above support the claim that the PR is mature. @kvid please share your thoughts on this :) thanks

awb1015 · 2021-10-13T16:24:52Z

Just an anecdotal comment, I had been using this branch (prior to rebase) almost exclusively for several months now without issue. Have now updated my local to be after the rebase and will comment if I have any issues.

A merge into dev would be greatly appreciated assuming everything is ok.

17o2 · 2021-10-13T17:18:09Z

Thanks @awb1015 for the feedback!

Usually I've waited for @kvid's approval before merging into dev, since he's quite good at catching edge cases, improving the code quality and providing generally valuable feedback. But as you can see by my previous comment, this might be a good PR to try out a new merging strategy, with post-hoc review and improvement.
I realize merging without a formal review would be an unilateral decision by me, going against the (spoken and unspoken) flow so far, and might be seen as hypocritical since kvid and others have had to wait for months for me to react to PRs and issues in the past, so I have been shy about this approach until now.

awb1015 · 2021-10-13T17:56:25Z

@formatc1702 No problem, definitely not trying to rush the process or skip any amount of review.

I have noticed over the last few hours, since moving over to the post-rebase version of the branch, that mating arrows between pairs of mating connectors aren't shown properly.

Prior to rebase I hadn't had any issues but now in the output .png I've noticed that the arrows between equivalent pins are not fully aligned to the row of the connector. Or in a few cases they're coming out from the corner of a connector and going to its mate's corner as well.

17o2 · 2021-10-13T18:03:37Z

Prior to rebase I hadn't had any issues but now in the output .png I've noticed that the arrows between equivalent pins are not fully aligned to the row of the connector. Or in a few cases their coming out from the corner of a connector and going to its mate's corner as well.

Could you provide some .png and the matching .yml and intermediate.gv files where you are having issues? Ideally both pre- and post-rebase, to make it easier. Using Gist might be easier to share the data than to paste it here directly.

awb1015 · 2021-10-14T14:57:17Z

I experimented with the YAML file after posting it and if I gave Connector 2 (both sides) a sequential pin list of 1,2,3,4,5,6 instead of 37,17,18,21,20,19 it seems like the issue went away. Obviously this isn't something I could do on actual diagrams as it changes the information being presented. However, I thought it might be helpful towards diagnosing the issue

17o2 · 2021-10-14T16:15:40Z

Please try the updated code; your bug should be fixed in 02a800a.

Root cause:

Use unique index for connector pin ports #229 changed the way connections between components are handled, by internally assigning unique IDs to the "ports" where wires (and now arrows) attach to connectors.
This mapping happens inside the Harness.connect() function, which is called when attaching wires.
Arrows between pins use Harness.add_mate_pin() instead, which did not implement the mapping
In addition, Harness.create_graph() had to be adjusted to add +1 to convert the [0-indexed] internal IDs to 1-indexed port IDs.
- The decision to use 1-indexed ports in GraphViz was taken so that port IDs would match the typical use case with connector pin numberings starting at 1
- This is also the reason why the bug only appeared with your custom pin numbering.

awb1015 · 2021-10-14T17:57:04Z

Looks good. Ran several dozens diagrams to confirm. Thank you for the update!

- Use pin names instead of pin indices, until the last moment when generating the ports for the GraphViz nodes - `Harness.add_mate_pin()` now uses pin names - Remove unused `if is_arrow()` check from `Harness.connect()` - Consolidate calling of `Connector.activate_pin()` to prevent subtle bugs - Call it from `connect()` and `add_mate_pin()` - No longer call it from `create_graph()` - Misc. other tuning

kvid

Sorry about this long period without any reply. These changes looks good. I only have a couple of suggestions.

src/wireviz/DataClasses.py

src/wireviz/wv_helper.py

17o2 · 2022-08-05T16:49:40Z

All comments have been addressed inside #251.

Not merging yet since it would immediately push dev forward, but without the latest code suggestions. I will wait until #251 is merged to have the suggestions make it directly into dev as well.

SeanDS · 2022-10-18T12:08:08Z

This looks promising for my use case. Would it be possible to group together the two sides of the mate in one box, instead of those arrows? I'm thinking a box with numbering on both left and right sides and labels between indicating the name of the pin pair. As I noted in #295 there are certain connectors where pin numberings change on the same conductor, so these numbers would ideally not be fixed.

Changes in wireviz#186 made it impossible to hide cable wire numbers. wireviz#186 (comment)

martinrieder · 2024-05-30T08:37:07Z

It seems that the user-configurable autogeneration character from 45bcc1d became an undocumented feature.

kvid · 2024-05-30T19:13:40Z

@martinrieder wrote:

It seems that the user-configurable autogeneration character from 45bcc1d became an undocumented feature.

You are right, it's not mentioned in the v0.4 docs, but I don't know if it might be due to lack of testing, unstability, side effects, just forgotten, or something else. Maybe @formatc1702 remembers any reason for omitting it from the docs?

Edit: If you @martinrieder have the time, maybe you could test this optional feature with a few different values, and verify the intended effect? Does it only work with a single character, or also with longer strings? Do you observe any unfortunate side-effects with certain characters/strings? Please store a few YAML input test cases for future regression testing.

17o2 · 2024-06-07T15:07:04Z

Thanks. I have added a line in the syntax description (in the dev branch) for now so it is not forgotten, see 5905041.

It's a good idea to do some testing as suggested; then the documentation can be expanded accordingly.

martinrieder · 2024-06-07T18:24:04Z

@kvid wrote:

Edit: If you @martinrieder have the time, maybe you could test this optional feature with a few different values, and verify the intended effect? Does it only work with a single character, or also with longer strings? Do you observe any unfortunate side-effects with certain characters/strings? Please store a few YAML input test cases for future regression testing.

I can do that. I guess that quotation characters could be problematic in this case. For sure, there should be a check to not accept an empty string here.

Refactor connector node generation Further refactor connector node generation Rebuild demos Generate gauge string inside Cable object WIP: refactor cable node generation Implement HTML indentation WIP More WIP Remove old stuff, slightly simplify code Outsource `gv_pin_table()`, simplify padding Add TODOs Outsource `set_dot_basics()` and `apply_dot_tweaks()` Make setting HTML tag attributes easier through `kwargs` Fix and simplify bgcolor logic Reactivate cable edge generation Outsource `gv_edge_wire()` Make connecting things more object-oriented Alphabetize HTML tags, improve bgcolor rendering Make mates object-oriented Run `autoflake -i` Run `autoflake -i --remove-all-unused-imports` Streamline assignment of ports to simple connectors Implement color objects Use color objects in WireViz Re-sort `wv_colors.py` Make green color darker Break longer lines not caught by `black` because they were unbroken strings or comments Make variable name more expressive Apply dot tweaks last Remove unused line Improve subclassing of components, prepare for BOM refactoring Clean up Include nested additional components in BOM do not add autogenerated designators to BOM Improve BOM generation (TODO: wires from a bundle) Prepare `harness.populate_bom()` Change `description` to `type` in additional BOM item YAML Define CLI epilog str in single statement Rename modules, adjust imports, move `build_examples.py` Restructure and update `.gitignore` Clarify `wireviz.parse()` input types Implement BOM population (missing: qty multipliers) Make `pin_objects` and `wire_objects` dictionaries Compute qty's of additional components (WIP) Add qty test file Adapt `tutorial08.yml` (remove `unit` field) Add `tabulate` to dependency list (might remove later if not needed) Sort BOM by category, assign BOM IDs Rename `Options.color_mode` to `.color_output_mod` for consistency Change BOM output file extension from `.bom.tsv` to `.tsv` Implement BOM bubbles Stop recursive nesting of additional components Add BOM bubble to additional component list (WIP) Fix gauge conversion Fix line breaks in code Optimize BOM bubble geometry Implement pin color output Small issue: GraphViz warning ``` Warning: table size too small for content ``` Add some test files to `tests/` directory Update test files Allow multiple colors for components Implement multiple colors for components, improve multicolor table rendering Fix color cell implementation Fix node background color rendering Add test file for node and title bgcolors WIP: BOM modes Add TODO for empty connector pin tables Comment out BOM modes (WIP) and BOM bubbles Resume work on BOM Include part number info in BOM table Fix BOM output in TSV and HTML Add bundles' wires' part number info to BOM Add TODOs Implement bundle part number rendering Improve conductor table rendering Fix additional component BOM table layout Disable CLI BOM output Add suggestions from #246 Add suggestions from #186 Add .vscode/ to .gitignore Fix PyLance problems Update interim version number Fix zero-size cell for simple connectors without type Implement additional parameters dict for components Implement note for additional components Thicken additional component table Add placeholder for add.comp. PN info Apply black

17o2 force-pushed the feature/mate+autogenerate branch from 0ed7c5d to 5a7c684 Compare October 22, 2020 16:49

17o2 added this to the v0.3 milestone Oct 22, 2020

17o2 force-pushed the feature/mate+autogenerate branch from 5a7c684 to 524b80c Compare October 22, 2020 20:43

17o2 added the WIP Work in progress label Oct 23, 2020

17o2 force-pushed the feature/mate+autogenerate branch from ea49e9a to 0c4b4bb Compare October 24, 2020 12:51

17o2 force-pushed the feature/mate+autogenerate branch from 23ec5c7 to 49a748f Compare November 1, 2020 14:53

17o2 closed this Nov 2, 2020

17o2 deleted the feature/mate+autogenerate branch November 2, 2020 09:37

17o2 restored the feature/mate+autogenerate branch November 2, 2020 09:38

17o2 mentioned this pull request Nov 10, 2020

Add option to add colors to connector pins #141

Merged

17o2 reopened this Nov 10, 2020

17o2 mentioned this pull request Nov 11, 2020

[Feature] Allow referencing a cable's/bundle's wires by color or label, if unambiguous #193

Closed

17o2 force-pushed the feature/mate+autogenerate branch from 49a748f to 83cb755 Compare November 14, 2020 09:03

17o2 mentioned this pull request Nov 14, 2020

[meta] Collaborator discussion #101

Open

17o2 force-pushed the feature/mate+autogenerate branch from 83cb755 to ecbc7d5 Compare November 15, 2020 08:18

17o2 marked this pull request as ready for review November 15, 2020 18:56

17o2 removed the WIP Work in progress label Nov 15, 2020

This was referenced Jan 1, 2021

How to create an alternating sides daisy chain without templates? #209

Closed

Error "first item not found anywhere" is unclear does not indicate the name of the item #207

Open

17o2 force-pushed the feature/mate+autogenerate branch from 0d1e3d7 to 74aeaa5 Compare September 23, 2021 13:16

17o2 requested a review from kvid September 23, 2021 15:10

17o2 modified the milestones: v0.3, v0.4 and later Oct 7, 2021

17o2 added 2 commits October 14, 2021 18:03

Fix bug of arrows using the wrong port IDs

02a800a

Simplify code

f0b63de

17o2 added the in latest label Oct 15, 2021

kvid requested changes Dec 18, 2021

View reviewed changes

src/wireviz/DataClasses.py Show resolved Hide resolved

src/wireviz/wv_helper.py Show resolved Hide resolved

17o2 mentioned this pull request Aug 5, 2022

Large scale refactoring #251

Open

5 tasks

17o2 added a commit that referenced this pull request Aug 5, 2022

Add suggestions from #186

d69e3ba

17o2 added a commit that referenced this pull request Aug 5, 2022

Add suggestions from #186

417c842

SeanDS mentioned this pull request Oct 18, 2022

Bulkhead connections #295

Closed

laurierloi pushed a commit to laurierloi/WireViz that referenced this pull request Jan 19, 2023

Add suggestions from wireviz#186

1faaff0

17o2 merged commit e3cc4c6 into dev Jun 7, 2023

17o2 deleted the feature/mate+autogenerate branch June 7, 2023 17:11

17o2 added a commit that referenced this pull request Jun 7, 2023

Add suggestions from #186

c3d7fb7

17o2 mentioned this pull request Jun 7, 2023

[feature] Improved auto-generated connectors and cables #184

Closed

kvid added a commit to kvid/WireViz that referenced this pull request Aug 27, 2023

Fix bug commented in wireviz#186

f9d1dd0

Changes in wireviz#186 made it impossible to hide cable wire numbers. wireviz#186 (comment)

kvid pushed a commit to kvid/WireViz that referenced this pull request Aug 27, 2023

Add suggestions from wireviz#186

8c7ef40

kvid pushed a commit to kvid/WireViz that referenced this pull request Sep 12, 2023

Add suggestions from wireviz#186

162fe44

17o2 mentioned this pull request Apr 15, 2024

[bug] Unconnected components are ignored by v0.4-dev #328

Closed

martinrieder mentioned this pull request May 17, 2024

[bug] Sanitize pin and wire labels so they are interpreted as strings #305

Open

martinrieder mentioned this pull request May 30, 2024

inline connectors or cables into connections? #259

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement mates between connectors, better autogeneration #186

Implement mates between connectors, better autogeneration #186

17o2 commented Oct 22, 2020 •

edited

Loading

17o2 commented Oct 24, 2020 •

edited

Loading

Test 1

Test 2

Test 3

Test 4

Test 9

17o2 commented Mar 20, 2021

17o2 commented Sep 23, 2021

awb1015 commented Oct 13, 2021

17o2 commented Oct 13, 2021 •

edited

Loading

awb1015 commented Oct 13, 2021 •

edited

Loading

17o2 commented Oct 13, 2021 •

edited

Loading

awb1015 commented Oct 14, 2021

17o2 commented Oct 14, 2021 •

edited

Loading

awb1015 commented Oct 14, 2021

kvid left a comment

17o2 commented Aug 5, 2022 •

edited

Loading

SeanDS commented Oct 18, 2022

martinrieder commented May 30, 2024

kvid commented May 30, 2024 •

edited

Loading

17o2 commented Jun 7, 2024

martinrieder commented Jun 7, 2024

Implement mates between connectors, better autogeneration #186

Implement mates between connectors, better autogeneration #186

Conversation

17o2 commented Oct 22, 2020 • edited Loading

17o2 commented Oct 24, 2020 • edited Loading

Test 1

Test 2

Test 3

Test 4

Test 9

17o2 commented Mar 20, 2021

17o2 commented Sep 23, 2021

awb1015 commented Oct 13, 2021

17o2 commented Oct 13, 2021 • edited Loading

awb1015 commented Oct 13, 2021 • edited Loading

17o2 commented Oct 13, 2021 • edited Loading

awb1015 commented Oct 14, 2021

17o2 commented Oct 14, 2021 • edited Loading

awb1015 commented Oct 14, 2021

kvid left a comment

Choose a reason for hiding this comment

17o2 commented Aug 5, 2022 • edited Loading

SeanDS commented Oct 18, 2022

martinrieder commented May 30, 2024

kvid commented May 30, 2024 • edited Loading

17o2 commented Jun 7, 2024

martinrieder commented Jun 7, 2024

17o2 commented Oct 22, 2020 •

edited

Loading

17o2 commented Oct 24, 2020 •

edited

Loading

17o2 commented Oct 13, 2021 •

edited

Loading

awb1015 commented Oct 13, 2021 •

edited

Loading

17o2 commented Oct 13, 2021 •

edited

Loading

17o2 commented Oct 14, 2021 •

edited

Loading

17o2 commented Aug 5, 2022 •

edited

Loading

kvid commented May 30, 2024 •

edited

Loading