Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Correctly check for show_wirenumbers default of None #221

Merged

Conversation

gopiballava
Copy link
Contributor

If show_wirenumbers is omitted from a cable section, its value will be None. In that case, we want to choose a default based on whether this is a bundle or not.

If the user did specify show_wirenumbers, then its value will be True or False, and we want to respect that whether it's a bundle or not.

If `show_wirenumbers` is omitted from a cable section, its value will be `None`. In that case, we want to choose a default based on whether this is a bundle or not.

If the user did specify `show_wirenumbers`, then its value will be `True` or `False`, and we want to respect that whether it's a bundle or not.
Copy link
Collaborator

@kvid kvid left a comment

Choose a reason for hiding this comment

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

I recommend accepting this PR based on a simple test with this YAML input:

cables:
  MyCable:
    colors: ['BN', 'RD', 'YE']
    show_wirenumbers: False

The left diagram shows that the result without this PR doesn't respect the show_wirenumbers: False for non-bundle cables, and the right diagram shows the result from the same YAML input after applying this PR:
issue221-before issue221-after
I have also verified that none of the generated example files change after applying this PR:

git checkout -b dev-built origin/dev
python build_examples.py
git add ../../{examples,tutorial}/*
git commit
python build_examples.py restore
git checkout gopiballava-fix-show_wirenumbers
python build_examples.py
python build_examples.py -b dev-built -c compare
python build_examples.py restore

@formatc1702 formatc1702 merged commit c0a885a into wireviz:dev Mar 20, 2021
formatc1702 added a commit that referenced this pull request Mar 20, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants