[meta] Collaborator discussion

[formatc1702]

No description provided.

[formatc1702]

I've created a new milestone to show which features I'd like to have included in the next release, i.e. pushing to master as well as uploading the new package to PyPI.

If anybody wants to suggest more issues to include, please let me know. However, my aim is to release v0.2 relatively soon, which is why I've deliberately left out a lot of existing requests.

[formatc1702]

Uptate on the v0.2 milestone:
I've assigned people for most open issues/PRs, hoping to be able to assign #114 soon too. If anybody wants to take on #60, please comment there and I will asign you.

Additionally, I've made a milestone for v0.3, which currently only serves to denote issues/PR drafts that I know I want to pursue as soon as v0.2 is out, but not before.

[kvid]

Have you considered #116 and #121 for the v0.2 milestone?

[formatc1702]

Added both! Thanks.

[kvid]

I just made a suggestion in the closed issue #122 (comment). Do you think it should be reopened?

[formatc1702]

Feel free to do so!

[kvid]

Feel free to do so!

I don't know how. Maybe I'm not authorized to do it.

[formatc1702]

Not sure.. did it myself now :)

Perhaps only repo owners (and maybe the original commenter) have the "reopen issue" button...

[kvid]

Not sure.. did it myself now :)

Thank's

Perhaps only repo owners (and maybe the original commenter) have the "reopen issue" button...

You are right. I certainly did not see that button here, but I can see it in one of the closed issues I opened myself.

[formatc1702]

Just a quick FYI especially for @kvid, I'll be taking a couple of days off coding to breathe a bit, and I promise I shall be back to read through all the comments, commits and other contributions as usual.
It will help me get back my focus and ensure v0.2 gets released with no major issues :)

[formatc1702]

My goal is to get v0.2 released by the end of the week.

• [feature] Include connector pinout diagrams #27 / Add optional image to connectors and cables #153 is almost ready for merging
• [feature] documentation on list of potential fields. #107 / Syntax documentation + new /docs directory #111 needs to be updated to reflect the above PR and is then ready, too.
• [feature] Only output the files that user requests #60 / [feature] CSV BOM output #98 / Modify CLI interface #123 have been postponed to v0.3 due to inactivity.
  I'm assuming this is due to the holiday period and I hope the authors make a comeback at some point :)
  Otherwise, I'm sure us other contributors will find a way to work on this while keeping the authorship intact.

"Perfect is the enemy of the good", so let's get this release out soon and hopefully receive feedback on the new features. I suggest that after the release, we can decide on prioritizing the upcoming issues to get another cool set of features for v0.3!

Thanks everybody who's contributed so far, with major kudos to @kvid for submitting good PRs and doing some thorough code review!

[formatc1702]

Unforeseen circumstances prevented me from working on WireViz this week, so the v0.2 release will have to wait another 2-3 weeks until after my holidays 😐 oh well...

[formatc1702]

Here's a quick update for everyone, especially for @kvid, @Tyler-Ward, @martonmiklos, @zombielinux [and others?] who have submitted contributions while I was gone. Thanks a lot for your continued support, and interest in this project!

I hope to resume reviewing/merging/coding in the upcoming weeks, so don't worry, this project is not forgotten!

Holidays, personal and work related issues have kept me busy and delayed things quite a bit... I will get back to every single issue and PR and integrate them into the project one by one as I find time to keep working on WireViz. I need a few more days to be able to continue, but rest assured, I am looking forward to integrate new features and get new releases out the door!
Cheers!

[kvid]

When the PRs are ready and unless rejected by @formatc1702 , I suggest he merge them into dev in this order: #115, #164, #163, #153, and #111, but please give the PR authors a couple of days for rebasing on top of the new dev and resolving conflicts before each merge.

[formatc1702]

Thanks for the suggestion. I haven't looked at the contributions in detail; just one question:

Why not prioritize #153 and #111, which were planned for v0.2 already, get that one released, and then continue with the other PRs you mention? I'm worried that the v0.2 release will be further delayed if we try to include every new addition, and it might be better to make a clean cut between what goes into v0.2, and what will go into v0.3 and beyond.

If there's a technical reason that would prevent this, please let me know. Thanks!

[kvid]

Thanks for the suggestion. I haven't looked at the contributions in detail; just one question:

Why not prioritize #153 and #111, which were planned for v0.2 already, get that one released, and then continue with the other PRs you mention? I'm worried that the v0.2 release will be further delayed if we try to include every new addition, and it might be better to make a clean cut between what goes into v0.2, and what will go into v0.3 and beyond.

I see your point.

If there's a technical reason that would prevent this, please let me know. Thanks!

• There are no critical reasons that I know about. My line of thought was this: As many more PRs are ready or close to ready now compared to before your vacation, I thought perhaps you considered moving more of them into v0.2. Syntax documentation + new /docs directory #111 already describes ignore_in_bom and @Tyler-Ward suggested you cold move that part from his PR Feature/additional components #115 into v0.2 - which is not a bad idea, but then during the last few days, the whole Feature/additional components #115 has moved very close to ready, and I thought you would be tempted to move the whole Feature/additional components #115 into v0.2.
• Given my assumptions, I suggested Feature/additional components #115 first, since that one include a major refactoring of BOM generation that will benefit other PRs.
• However, as my assumptions that you maybe wanted to move new PRs into v0.2 was wrong, then Add optional image to connectors and cables #153 and Syntax documentation + new /docs directory #111 should be merged in first, and maybe @Tyler-Ward need more time than I think to get Feature/additional components #115 ready (mostly because of my change suggestions), but after it gets ready, I hope you consider Feature/additional components #115 soon rather than later. 😃
• Update the types of dataclass attributes according to usage #163 is kind of coupled with a Syntax documentation + new /docs directory #111 suggestion, but it should be possible to accept my Syntax documentation + new /docs directory #111 (comment) even before Update the types of dataclass attributes according to usage #163 is merged in.
• Remove HTML links from the input attributes  #164 is quite small, but potentially very useful for many users.

[formatc1702]

Thanks for the detailed answer.

• I will stick to finishing v0.2 with Add optional image to connectors and cables #153 and Syntax documentation + new /docs directory #111 first, to avoid "feature creep".
• I will then follow your other suggestions, and I see no reason not to release v0.3 soon afterwards... Since you've had a good eye on the project recently, I'm sure you can help pick out the PRs that are close to completion, and any burning issues that should be resolved and included in v0.3 as well.

There's tons of new stuff in v0.2 already, compared to the current master branch, so it's nice to keep some good stuff for the following release ;)

[stevegt]

@formatc1702 Let us know how we can help. I'm extremely impressed by how motivated and active the contributors to this project have been in its short life, and I'd like to do what I can to ensure its continuity. I'm already pretty sure the WireViz yaml schema could become a defacto industry standard, though that does raise the thorny issue of governance -- maybe some sort of RFC or PEP-like approach might work. I'm also thinking of suggesting you get on the waiting list for the github Sponsors program if you haven't already.

[formatc1702]

@formatc1702 Let us know how we can help. I'm extremely impressed by how motivated and active the contributors to this project have been in its short life, and I'd like to do what I can to ensure its continuity. I'm already pretty sure the WireViz yaml schema could become a defacto industry standard, though that does raise the thorny issue of governance -- maybe some sort of RFC or PEP-like approach might work. I'm also thinking of suggesting you get on the waiting list for the github Sponsors program if you haven't already.

Thanks @stevegt for the kind words! It has been amazing to see the project grow, and indeed, collaboration with all the contributors has been great!

I'll lookinto the GitHub Sponsors thing; however, I'm unsure whether monetizing the project in this way might negatively impact users' expectations, and contributors' attitudes, since a lot of WireViz is built on their work, not only mine.

[formatc1702]

Let us know how we can help.

Since my time for working on the project has been a bit limited lately, the following things have helped immensely:

• Contributors submitting high quality PRs, possibly rebasing onto dev if necessary, so all I need to do is nitpick a bit and click Merge ;)
• Contributors collaborating without my intervention and checking each others work (e.g. in Feature/additional components #115), leading to the aforementioned quality
• @kvid keeping an overview of the various issues, how they interact, and suggesting an order in which to include them; as well as meticulously checking for uncaught mistakes and ensuring completeness of documentation, etc.

This is all highly appreciated, and I hope we can keep the momentum of the project going :)

[formatc1702]

Posting it here as well for completeness sake:
v0.2 has been released, thank you everybody for your valuable contributions!

Over 100 forks and >2000 stars on the project really tell me we're working on something worthwhile here!

[formatc1702]

Short summary of the current situation:

• Move BOM management and HTML functions to separate modules #192 seems to be working, will re-review it soon and merge
• I am currently working on Implement mates between connectors, better autogeneration #186 and hope to have a version that is ready for review soon-ish
• Continuous documentation #181 has been updated to reflect the recently merged PRs
• I could use some help with the following:
  • Getting add length_unit support for wire and cable lengths #171 ready for merge (high priority, has been waiting for a while)
  • Getting Add href attribute to connectors, cables, and additional_bom_items #168 ready for merge
  • Embed images in SVG output #189 might spawn a new PR for embedding images in the SVG/HTML output Embed images in SVG output #189 now includes a WIP of SVG image embedding
    • I could tackle that after finishing the above PRs, but I will also accept contributions if anybody feels up for the task

As you can see, I'd like to reduce the number of open PRs first. Afterwards, I'll try and prioritize some issues that deserve attention :-)

[formatc1702]

@kvid if you could let me know what order the mentioned PRs should be merged, and whether you need to do anything on them, that would help me to integrate them in the most painless way possible :) Thanks.

[kvid]

As #197 and #229 are already merged, #214 and #219 can follow in that order. I have rebased them both on top of the current dev. Please review them (anyone?) to increase the quality before merging. It should be possible to merge #229 both before or after them, but I have not checked for conflicts. I experience a lot of conflicts with import statements while rebasing. I think we should consider splitting imports into several lines (one identifier per line) and sort them alphabetically per import statement to reduce conflicts. Maybe there is a PEP describing this?

[kvid]

I think this project follows PEP8 import guidelines more or less, but I think we should also consider sorting imports according to some common rules to reduce merge conflicts. Maybe a tool like isort can be used to automate the task? Some projects also combine isort with black to force a fully unified code formatting on all Python files. Please tell about your own experiences with formatting rules and tools like this.

[formatc1702]

It will soon be a year since v0.2 of WireViz was released. There are some rather big PRs pending, but I think that the dev branch has been in a very good shape recently (the only known major bug is #220, and a solution is on the way) so I have decided I will release v0.3 soon as it is in the current state because it includes some important features that I don't want to hold back any longer.

[formatc1702]

I experience a lot of conflicts with import statements while rebasing. I think we should consider splitting imports into several lines (one identifier per line) and sort them alphabetically per import statement to reduce conflicts. Maybe there is a PEP describing this?

I think we should also consider sorting imports according to some common rules to reduce merge conflicts

Recently, I have tried to stick to the following scheme and am trying to make this consistent across files.
Packages sorted alphabetically per block, and individual modules sorted alphabetically within a package's line.

# built-in packages
import os
from pathlib import Path
import sys
# ...
        # blank line
# external dependencies
from graphviz import Graph
import yaml
# ...
        # blank line
# internal modules
from wireviz.DataClasses import Connector, Cable, Metadata, Options, Tweak
from wireviz.wv_colors import get_color_hex, translate_color
# ...

There will surely be some conflicts in the near future, but IMHO it is a clean and structured guideline that should reduce conflicts in the future.

[amotl]

Dear Daniel,

we can warmly recommend using the excellent isort and black programs from the Python ecosystem to bring the codebase into a standards-based shape on those matters.

With kind regards,
Andreas.

P.S.: Ah, I just recognized that @kvid already recommended those programs just two posts above, at #101 (comment). Apologies.

[formatc1702]

Thanks @kvid and @amotl for the isort and black suggestions! I am trying them out and I have to say, they look really promising!
Do you have experience with PRs branching off before reformatting with black, and attempting to merge after reformatting? Is it reasonable to expect that merging will go smoothly if both the base branch and the PR branch are reformatted independently?

[amotl]

Hi again,

I just wanted to leave another note on the isort/black thing. With earthobservations/gribmagic@0d795ea, I just recently added it to another project and found the --profile=black option of isort sensible.

isort --profile=black foobar
black foobar

This option is important to make isort's formatting of imports compatible with Black's code-formatting style - see also [1].

Do you have experience with PRs branching off before reformatting with black, and attempting to merge after reformatting? Is it reasonable to expect that merging will go smoothly if both the base branch and the PR branch are reformatted independently?

If both branches have been formatted with the same options, I believe it is reasonable to expect that merges will apply well without further ado.

With kind regards,
Andreas.

[1] https://pycqa.github.io/isort/docs/configuration/black_compatibility.html

[formatc1702]

WireViz v0.3 is released 🎉

And we already have a bunch of very interesting PRs that could warrant a v0.4 not long after!

[formatc1702]

I have rebased some of the open PRs on top of each other, in order to be able to have a latest branch that incorporates all the changes, while still keeping each individual PR open for review before inclusion into dev.

I encourage any users wishing to test the features in the following PRs to use the latest branch and try them out!

• Implement mates between connectors, better autogeneration #186
• Improve output of diagram + BOM as technical drawing #239
• Replace os.path with pathlib.Path #246 (internal improvement only)
• Clean up and improve CLI implementation #244
• Apply isort and black to source code #248
• Improve wireviz.parse() #250
• Embed images in SVG output #189

Bear in mind that this branch might not be as stable as dev. Please report any bugs in the corresponding PR. If in doubt of which of them is causing the issue, you may also open a new issue.
Ideally, any code reviews and approvals of the PRs should happen in the order of the list above. This way, the dev branch can catch up with latest step by step.

Due to the changes introduced in #244, a fresh install of WireViz using pip is recommended, since the CLI parsing is now handled by wv_cli.py, and the CLI entry points have therefore moved.

I am sorry for any inconvenience caused by force-pushing the PR branches after rebasing. However, I believe this is the best approach to allow me to continue developing the project freely and on top of all the latest features, without compromising the relative stability of the dev branch. Additionally, every time I run WireViz from latest, I am essentially testing all the above PRs simultaneously, so any bugs can hopefully be caught faster.

I welcome any comments on this new approach.

[amotl]

I welcome any comments on this new approach.

I believe it is a good decision in order to move forward. Thanks a stack for your work and keep up the spirit!

[kvid]

I just noticed a hotfix is considered, as described in issue #254 (comment). If that will be released, I suggest also adding to the syntax description a simple warning that states all bgcolor_* attributes (and possibly some bgcolor attributes) are experimental and might get a different syntax in future releases. A future merge of #255 will break this syntax, and I recommend a warning about this.

[formatc1702]

I just noticed a hotfix is considered, as described in issue #254 (comment). If that will be released, I suggest also adding to the syntax description a simple warning that states all bgcolor_* attributes (and possibly some bgcolor attributes) are experimental and might get a different syntax in future releases. A future merge of #255 will break this syntax, and I recommend a warning about this.

There will certainly be some more syntax changes in the future, such as for autogeneration of components (#186), potentially renaming connector.style to connector.category (as propsed here), and maybe others.

There is also the general warning in the README concerning this:

This is very much a work in progress. Source code, API, syntax and functionality may change wildly at any time.

Therefore, I see no need for an additional warning here.

[martonmiklos]

Hi folks,

I am trying to get the #189 working.
What is the status of the latest branch?

I got the following error:

mm@mm-precision:~/Dokumentumok/BlackEye/loopback/drawings/WireViz$ wireviz ../h/h.yaml 

WireViz 0.4-dev
Input file:   ../h/h.yaml
Output file:  ../h/h.[html|png|svg|tsv]
Traceback (most recent call last):
  File "/home/mm/.local/bin/wireviz", line 11, in <module>
    load_entry_point('wireviz', 'console_scripts', 'wireviz')()
  File "/usr/lib/python3/dist-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python3/dist-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python3/dist-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/home/mm/Dokumentumok/BlackEye/loopback/drawings/WireViz/src/wireviz/wv_cli.py", line 137, in wireviz
    wv.parse(
  File "/home/mm/Dokumentumok/BlackEye/loopback/drawings/WireViz/src/wireviz/wireviz.py", line 88, in parse
    yaml_data, yaml_file = _get_yaml_data_and_path(inp)
  File "/home/mm/Dokumentumok/BlackEye/loopback/drawings/WireViz/src/wireviz/wireviz.py", line 397, in _get_yaml_data_and_path
    raise e
  File "/home/mm/Dokumentumok/BlackEye/loopback/drawings/WireViz/src/wireviz/wireviz.py", line 389, in _get_yaml_data_and_path
    yaml_path = Path(inp).expanduser().resolve(strict=True)
  File "/usr/lib/python3.8/pathlib.py", line 1181, in resolve
    s = self._flavour.resolve(self, strict=strict)
  File "/usr/lib/python3.8/pathlib.py", line 363, in resolve
    return _resolve(base, str(path)) or sep
  File "/usr/lib/python3.8/pathlib.py", line 347, in _resolve
    target = accessor.readlink(newpath)
  File "/usr/lib/python3.8/pathlib.py", line 452, in readlink
    return os.readlink(path)
OSError: [Errno 36] File name too long: "/home/mm/Dokumentumok/BlackEye/loopback/drawings/WireViz/ \nconnectors:\n  'aand here comes the YAML content

It looks that some paramteres swapped. Is it worth troubleshooting it or is there a better branch to start with the SVG embed?

[jakew009]

Same issue as @martonmiklos

[kvid]

I've not tested the #189 code myself after it was rebased into the latest chain of PR branches, but the #189 (comment) explains that there exists an alternative branch backup/bugfix/image-src_before-latest-rebase that might help you.

Either the alternative branch can be used directly, or the two branches can be compared to investigate differences.

[amotl]

Dear Daniel,

it is sweet to see you working on #186 and #251 ff. again. Just let me know when you converged this into a new release, and I will update WireViz-Web correspondingly.

With kind regards,
Andreas.

[kvid]

My contributions to help with filalizing #251 is work in slow progress due to an increase in the effort I have to use on other important stuff in my life this autumn. Hence, I propose to consider creating a release 0.4 BEFORE merging in #251 - to let all users try out the new functionality now, including other projects that depend on this projepct. It has already been way too long since our last release. There is no problem also adding an extra release shortly after merging in #251 if needed.

I've already made an effort on this code by merging in a few extra PRs after the huge effort by @formatc1702 this summer, and fixing some minor stuff, so generating examples should work now.

I encourage @amotl and also other motivated users/contributors to try out the current dev branch and look out for minor things that are easy to fix before a release, like e.g. any missing or wrong documentation of new features. I also consider merging #298 into dev before such a release, as it is a low hanging fruit and should be easy to integrate with #251 later.

Edit: Should we also consider to merge in #309 (maybe after some adjustments) to make the release I suggest more up-to-date on the Python versions in use today?

[formatc1702]

I very much support a release of v0.4 before #251. Including #298 should be no problem. I can take care of publishing onto PyPI once I get the signal.

Adding support for current Python versions (#309), and even dropping support for deprecated ones, should help in keeping the project fresh and not bound to older syntax conventions etc. I have not been actively following the updates but I do not want the project to be bloated by feeling forced to support old versions.

Thank you for pushing forward, but don't let the project get in the way of life either! 💪

[kvid]

IMHO, we need to fix #328 before a new release. Please comment my suggested fix with a new option in that issue.

[formatc1702]

I have submitted #332 as a response to #328.

I would like to go forward with merging #332 and #298 into dev, publishing v0.4 and afterwards immediately rebasing big-refactor onto dev and fast-fowarding, so finally we have a dev branch with more manageable PRs again.

[formatc1702]

I have merged #332 and #298, updated the version number and added a v0.4-rc tag.

[Edit 2024-05-05]: Renamed tag to v0.4-rc to distinguish from proper v0.4 once it is merged into master.

However, I consider this an interim version that will not be merged into master or published on PyPI, and the changelog reflects this.

The next immediate step is rebasing #251 onto the latest dev branch, now labeled v0.5-dev to allow differentiating between changes from before (v0.4) and after (v0.5) the refactoring. I would appreciate any help in doing so. Then dev can be moved forward to contain the refactored code to finally get rid of big-refactor even if not all kinks are fully ironed out yet.

[kvid]

I have merged #332 and #298, updated the version number and added a v0.4 tag.

Thank you!

However, I consider this an interim version that will not be merged into master or published on PyPI, and the changelog reflects this.

I don't understand why v0.4 should not be merged into master and released the normal way. My main motivation for suggesting v0.4 before merging #251 was to make it available to users and getting more usage feed-back.

It shouldn't be much work taking this remaining step to a full release, and I see no drawbacks about it, unless you know about any critical bugs or documentation flaws in v0.4.

The next immediate step is rebasing #251 onto the latest dev branch, now labeled v0.5-dev to allow differentiating between changes from before (v0.4) and after (v0.5) the refactoring. I would appreciate any help in doing so. Then dev can be moved forward to contain the refactored code to finally get rid of big-refactor even if not all kinks are fully ironed out yet.

I agree to this step, but AFTER a merge of v0.4 into master.

[martinrieder]

@kvid @formatc1702
I see so many feature requests and I would even have some more ideas... It seems overwhelming and possibly requires some priorisation or roadmapping.
I suggest that one of you starts an issue for discussion on this topic. Similar to #365, there should be some PR about the next major release already. I do not want to take ownership of it, though. ;)

It is not obvious, which PRs should be based on master or dev. I noticed that PR #357 is based on the dev branch. The others are merged into a release branch that is directly based on master. If I am to prepare some PR, what shall be the base?

[kvid]

I use #365 to collect simple bugfixes to be released soon, and I have chosen to base such PRs on master to enable other PRs (not part of the upcoming bugfix release) being based on dev in parallel. The next PR to be merged into dev is #251, but I've been too busy lately to look into that yet.

However, it's normally trivial to rebase a PR branch from dev to master or vice versa (unless the amount of change conflicts is huge) if priorities are changed. We are all free to argue for which PRs to include in a release, and if you feel e.g. PR #357 is important to release before a future v0.5, then please describe your reasons for that in a comment. 😃

Normally, I recommend basing a new PR on dev, but if it's a bugfix or other minor change with high priority, then you probably should argue for basing it on master.

[martinrieder]

@kvid wrote:

I use #365 to collect simple bugfixes to be released soon, and I have chosen to base such PRs on master to enable other PRs (not part of the upcoming bugfix release) being based on dev in parallel. The next PR to be merged into dev is #251, but I've been too busy lately to look into that yet.

Dito. I get the impression, #251 would definitely demand an 0.5 release branch or even a major step to 1.0...
I would love to see semantic-release in action by then. There is a python-specific version called python-semantic-release...

We are all free to argue for which PRs to include in a release, and if you feel e.g. PR #357 is important to release before a future v0.5, then please describe your reasons for that in a comment. 😃

That is the reason why Semantic Versioning states clear rules and leaves out the personal feelings. If we were to follow these, #357 is a feature that would be part of 0.5.

Normally, I recommend basing a new PR on dev, but if it's a bugfix or other minor change with high priority, then you probably should argue for basing it on master.

There is another argument for basing it on master in the GitHub Docs about tracking issues and PRs:

The special keywords in a pull request description are interpreted when the pull request targets the repository's default branch. However, if the PR's base is any other branch, then these keywords are ignored, no links are created and merging the PR has no effect on the issues. If you want to link a pull request to an issue using a keyword, the PR must be on the default branch.

Note that I do not intend to influence the way that you have been working on this project. I am just giving hints on what could be improved. As long as you can handle things manually and there is no disagreement amongst the collaborators, just keep going without semantic-release. It simplifies and automates some chores, but it also adds dependencies, complexity and thereby limits the freedom that an innovative project needs in early stages!

My actual intent was only to ask for a list of feature priorities for the next release, possibly discussed in a dedicated issue! If you think that this issue is the right place to discuss this, then I am also fine with it.

EDIT: I found this #320 (comment) by coincidence, asking for help on untangling #251 commits.

[kvid]

@formatc1702, @amotl, @martinrieder - Please consider my suggested way to merge as described in #408 (comment) with hight priority, or suggest alternatives.