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

Empty values within FIELDS identified in %values sections #144

Closed
wetzelj opened this issue Sep 9, 2020 · 3 comments
Closed

Empty values within FIELDS identified in %values sections #144

wetzelj opened this issue Sep 9, 2020 · 3 comments

Comments

@wetzelj
Copy link
Contributor

wetzelj commented Sep 9, 2020

This bug presents itself in two locations:

  1. When a %values lists contains a FIELD which has an empty value, the empty string is used to search for fields on which to act in the action. This causes all fields to be identified and all header fields to be removed.
  2. When a %values lists contains only FIELDS/SPLITS which result in empty values within the values list, the empty list is used to when searching for fields to target and results in the removal of all header fields.

Values being added to a %values list should be interrogated so that empty strings are not added to values lists and %values lists should be interrogated so that if a values list contains no values, the action is ignored.

I've put together a possible fix as well as unit tests for this condition within my fix/empty_values_lists branch. Let me know what you think.

@wetzelj
Copy link
Contributor Author

wetzelj commented Sep 9, 2020

FYI... I recently updated to black v20.8b1. It appears to have introduced some changes to the docstring quotes. If there's a specific version I should be using, let me know. :)

@vsoch
Copy link
Member

vsoch commented Sep 9, 2020

Your branch looks great! One suggestion, it's slightly more python to do:

if values:

instead of

if len(values) > 0:

For black we install the latest version, so it might be okay (we can see how it runs when the PR is opened). We can pin a version if necessary after that. Open away! :)

@wetzelj wetzelj mentioned this issue Sep 9, 2020
3 tasks
@wetzelj wetzelj closed this as completed Sep 11, 2020
@vsoch
Copy link
Member

vsoch commented Sep 11, 2020

Thank you again for this! It’s really lovely to have someone to collaborate with, and fulfilling to see the library getting better.

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

No branches or pull requests

2 participants