Skip to content

Conversation

@desilinguist
Copy link
Collaborator

This PR closes #693.

  • Raise an exception if —drop_blanks removes all rows in the input file if each of them contains a blank value.
  • Add a test for this scenario.
  • Update —help output for filter_features to make it clear that —drop_blanks and —replace_blanks_with only apply to CSV/TSV files.

It makes sense to raise an exception as soon as we detect this since futher processing with an empty features array will run into additional issues.
Make it clear that `--drop_blanks` and `--replace_blanks_with` only apply to CSV/TSV files.
This test checks that an exception is correctly raised if `--drop_blanks` removes all rows in the input file.
@codecov
Copy link

codecov bot commented Dec 7, 2021

Codecov Report

Merging #703 (3a697b6) into main (a5d5b3f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #703   +/-   ##
=======================================
  Coverage   96.89%   96.89%           
=======================================
  Files          63       63           
  Lines        9197     9213   +16     
=======================================
+ Hits         8911     8927   +16     
  Misses        286      286           
Impacted Files Coverage Δ
skll/utils/commandline/filter_features.py 98.48% <ø> (ø)
skll/data/readers.py 90.26% <100.00%> (+0.06%) ⬆️
tests/test_commandline_utils.py 99.67% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5d5b3f...3a697b6. Read the comment docs.

@desilinguist
Copy link
Collaborator Author

The code coverage issue can be ignored.

@desilinguist desilinguist requested a review from a user December 7, 2021 15:11
@desilinguist desilinguist merged commit ede139e into main Dec 7, 2021
@delete-merged-branch delete-merged-branch bot deleted the 693-fix-drop-blanks-for-filter-features branch December 7, 2021 16:36
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.

--drop-blanks in filter_features failes if every single row in the feature file has a blank

5 participants