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

[FIX] OWTable: Sort Continuous metas as floats; not strings #1678

Merged
merged 1 commit into from
Oct 21, 2016

Conversation

nikicc
Copy link
Contributor

@nikicc nikicc commented Oct 20, 2016

Continuous variables inside meta attributes are currently sorted by their string represenation and not by float values. This happens since dtype for metas is object and we map all object to strings before sorting. This PR checks whether the sorting variable is ContinuousVariable and casts values to floats.

@codecov-io
Copy link

codecov-io commented Oct 20, 2016

Current coverage is 89.38% (diff: 100%)

Merging #1678 into master will not change coverage

@@             master      #1678   diff @@
==========================================
  Files            79         79          
  Lines          8593       8593          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           7681       7681          
  Misses          912        912          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update e9d3332...4f9931e

@@ -1015,6 +1015,8 @@ def columnSortKeyData(self, column, role):
and role == TableModel.ValueRole:
col_view, _ = self.source.get_column_view(coldesc.var)
col_data = numpy.asarray(col_view)
if coldesc.var.is_continuous: # continuous from metas have dtype object; cast it to float
Copy link
Contributor

Choose a reason for hiding this comment

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

Please avoid comments on the same line when that results in line length > 80.

Continuous variables inside meta attributes are currently sorted by their string represenation and not by float values. This happens since dtype for metas is object and we map all object to strings before sorting. This PR checks whether the sorting variable is ContinuousVariable and casts values to floats.
@nikicc
Copy link
Contributor Author

nikicc commented Oct 21, 2016

@lanzagar I moved it inside the if. Is this better?

@lanzagar
Copy link
Contributor

It is.

@lanzagar lanzagar merged commit 5899ce2 into biolab:master Oct 21, 2016
@nikicc nikicc deleted the continuous-metas-sorting branch October 21, 2016 09:55
@astaric astaric modified the milestone: 3.3.9 Nov 28, 2016
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.

4 participants