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.

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

Correctly check for show_wirenumbers default of None #221

Merged
merged 1 commit into from
Mar 20, 2021

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

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

Successfully merging this pull request may close these issues.

3 participants