Skip to content

Conversation

@janezd
Copy link
Contributor

@janezd janezd commented Jan 12, 2018

Based on #2866 (1 commit). You may want to merge that request first.

Issue

#2866 fixes an issue with labels in MDS that occured since self.data.domain was False for a Domain with only meta attributes. This occurs since Domain.__len__ (correctly) returns the number of elements that Domain.__iter__ iterates across, however, Domain.__iter__ (incorrectly) ignores metas.

The issue will remain even after we fix Domain.__iter__: when checking whether a domain is true, we usually mean whether it's not None.

Description of changes

Domain has a new method, empty.

__bool__ warns that it's ambiguous and suggests using either empty or is None. The behaviour of __bool__ is unchanged - it returns True if the lenght of the domain is non-zero.

Includes
  • Code changes
  • Tests

@janezd janezd requested a review from astaric January 12, 2018 11:06
@janezd janezd force-pushed the obsolete-domain-bool branch 3 times, most recently from 1dadff7 to 83ab794 Compare January 12, 2018 16:15
@codecov-io
Copy link

codecov-io commented Jan 12, 2018

Codecov Report

Merging #2867 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #2867   +/-   ##
======================================
  Coverage    82.1%   82.1%           
======================================
  Files         328     329    +1     
  Lines       56210   56210           
======================================
  Hits        46151   46151           
  Misses      10059   10059

@lanzagar lanzagar added this to the 3.10 milestone Jan 19, 2018
@lanzagar
Copy link
Contributor

Rebase please.

@janezd janezd force-pushed the obsolete-domain-bool branch from 83ab794 to 3e41036 Compare January 19, 2018 13:09
@lanzagar lanzagar merged commit 3742991 into biolab:master Jan 19, 2018
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