Skip to content

Commit

Permalink
Refactor #55
Browse files Browse the repository at this point in the history
  • Loading branch information
@formatc1702
formatc1702 committed Jul 4, 2020
1 parent ebf1e5a commit 144c99e
Showing 1 changed file with 9 additions and 10 deletions.
19 changes: 9 additions & 10 deletions src/wireviz/wireviz.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,17 +197,16 @@ def check_designators(what, where):
harness.output(filename=file_out, fmt=('png', 'svg'), gen_bom=generate_bom, view=False)

if return_types is not None:
# gather the harness data and return it as a single data type
if isinstance(return_types, str):
if return_types.lower() == 'png':
return harness.png
returns = []
if isinstance(return_types, str): # only one return type speficied
return_types = [return_types]

else:
# gather the harness data and return it as a series of tuples
returns = []
if 'png' in return_types:
returns.append(harness.png)
return tuple(returns)
return_types = [t.lower() for t in return_types]

if 'png' in return_types:
returns.append(harness.png)

return tuple(returns) if len(returns) != 1 else returns


def parse_file(yaml_file, file_out=None, generate_bom=False):
Expand Down

6 comments on commit 144c99e

@slightlynybbled
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this change was supposed to do. The pull request that I provided to you worked and now I can't seem to get it to work...

data = parse(f_in, return_types='png')

Did you test this? I can't seem to get it to return the data correctly...

@slightlynybbled
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, you took out a feature...

I had to change

data = parse(f_in, return_types='png')

to this

data = parse(f_in, return_types=('png', ))[0]

This is acceptable, but complicates the call for anyone else that might only have one type of output (most common)

@formatc1702
Copy link
Collaborator Author

@formatc1702 formatc1702 commented on 144c99e Jul 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The last line should have been

        return tuple(returns) if len(returns) != 1 else returns[0]

to have the desired behavior, sorry.

The idea was to make it easier to expand the code for more output formats

  • Remove duplicate calls of harness.png (line 203 and 209 in original file)
  • The second call did not invoke the lower() function, which was not consistent.. this way, it's done once and for all :)

Please try the new code (de1e45f), I've added SVG output to test the case of more than one return_type

@slightlynybbled
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, good deal. That meets the intent of what I was looking for.

Any idea when the dev branch will be included in the main branch? I don't want to build too much an an immature API, but I don't want to hold you back either. Right now, I'm basically including the dev-branched copy in my code directly.

@formatc1702
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For now, I think it's best to keep using the dev branch, since the project is in an early stage, and there's no real meaning to a 'master' release yet, anyway.
I doubt the API your GUI uses will change much, but

  1. who knows, and
  2. the WireViz syntax might also change, maybe radically, e.g. if we need finer control over a cable's individual wire parameters, see [feature] More control over wire parameters #56 for example, and we realize the current syntax doesn't allow this.

So, thanks a lot for your effort in building a WireViz GUI, just be aware of the above points! 😃

@slightlynybbled
Copy link
Contributor

Choose a reason for hiding this comment

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

Eagerly awaiting the the next release, not going to do much to build on until then...

Please sign in to comment.