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

Change None values to none strings #1442

Merged
merged 2 commits into from
Feb 3, 2024
Merged

Change None values to none strings #1442

merged 2 commits into from
Feb 3, 2024

Conversation

nfahlgren
Copy link
Member

@nfahlgren nfahlgren commented Jan 30, 2024

Describe your changes
In the function plantcv.plantcv.utils.json2csv the unique values for a list of metadata values is computed but fails when one or more of the values is None. This PR changes the None values to a "none" string, which is the default metadata value.

Thanks to @leowlima for reporting the issue!

Type of update
Is this a: Bug fix

Additional context
A better long-term solution would be to set the values to the default value instead of None.

For the reviewer
See this page for instructions on how to review the pull request.

  • PR functionality reviewed in a Jupyter Notebook
  • All tests pass
  • Test coverage remains 100%
  • Documentation tested
  • New documentation pages added to plantcv/mkdocs.yml
  • Changes to function input/output signatures added to updating.md
  • Code reviewed
  • PR approved

Fixes issue where the unique values of an array cannot be calculated when one or more values is None
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (339fb09) 99.98% compared to head (33da64b) 99.98%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1442   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         158      158           
  Lines        6973     6975    +2     
=======================================
+ Hits         6972     6974    +2     
  Misses          1        1           
Flag Coverage Δ
unittests 99.98% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
plantcv/utils/converters.py 100.00% <100.00%> (ø)

@annacasto annacasto requested review from annacasto and removed request for annacasto February 2, 2024 16:45
@nfahlgren nfahlgren merged commit ecdfbf4 into main Feb 3, 2024
6 checks passed
@nfahlgren nfahlgren deleted the fix-unique-none-list branch February 3, 2024 02:15
@nfahlgren nfahlgren added this to the PlantCV v4.3 milestone Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants