Skip to content

astype(bool) was changing 'False' to True. Fixed#33

Merged
joeyshuttleworth merged 6 commits intomainfrom
hilary_dev
Oct 25, 2024
Merged

astype(bool) was changing 'False' to True. Fixed#33
joeyshuttleworth merged 6 commits intomainfrom
hilary_dev

Conversation

@hilaryh
Copy link
Copy Markdown
Contributor

@hilaryh hilaryh commented Oct 7, 2024

Description

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Testing

  • Testing is done automatically and codecov shows test coverage
  • This cannot be tested automatically

Documentation checklist

  • I have updated all documentation in the code where necessary.
  • I have checked spelling in all (new) comments and documentation.
  • I have added a note to RELEASE.md if relevant (new feature, breaking change, or notable bug fix).

Copy link
Copy Markdown
Collaborator

@joeyshuttleworth joeyshuttleworth left a comment

Choose a reason for hiding this comment

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

Extra whitespace added to L407

@joeyshuttleworth
Copy link
Copy Markdown
Collaborator

The bug could be to do with a type issue where there are missing values in an otherwise boolean column causing pandas to treat the column as a string where bool('False') is True.

@joeyshuttleworth
Copy link
Copy Markdown
Collaborator

I think you need to use 'apply' not 'map' ?

@hilaryh
Copy link
Copy Markdown
Contributor Author

hilaryh commented Oct 25, 2024

I think you need to use 'apply' not 'map' ?

Either could work - but map does an element-wise check whereas apply would go column by column so it'd be something longer like
qc_df[qc_criteria].apply(lambda y: [x=='True' or x==True for x in y]), right?

@joeyshuttleworth joeyshuttleworth merged commit cee71fd into main Oct 25, 2024
@mirams mirams deleted the hilary_dev branch October 25, 2024 16:08
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