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

[ENH] tSNE: Output preprocessor #3407

Merged
merged 1 commit into from
Nov 23, 2018
Merged

Conversation

VesnaT
Copy link
Contributor

@VesnaT VesnaT commented Nov 21, 2018

Issue

Implements third part of #3342

Description of changes
Includes
  • Code changes
  • Tests
  • Documentation

@codecov
Copy link

codecov bot commented Nov 21, 2018

Codecov Report

Merging #3407 into master will decrease coverage by <.01%.
The diff coverage is 98.61%.

@@            Coverage Diff             @@
##           master    #3407      +/-   ##
==========================================
- Coverage   82.33%   82.32%   -0.01%     
==========================================
  Files         360      360              
  Lines       64045    64075      +30     
==========================================
+ Hits        52729    52751      +22     
- Misses      11316    11324       +8

@VesnaT VesnaT changed the title tSNE: Output preprocessor [ENH] tSNE: Output preprocessor Nov 21, 2018
@janezd janezd assigned janezd and pavlin-policar and unassigned janezd Nov 23, 2018
tsne_cols = [ContinuousVariable('t-SNE-%d' % (i + 1))
for i in range(self.tsne.n_components)]
n = self.tsne.n_components
postfixes = ['x', 'y'] if n == 2 else list(range(n))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like list(range(n)). While it would very rarely come up, it would imply that the variables are named t-SNE-0, t-SNE-1, t-SNE-2, ... I guess it is consistent with the current behaviour of Manifold learning, which outputs C0, C1. Honestly, I think t-SNE-x and t-SNE-y should be removed in favour of t-SNE-1, t-SNE-2, and if the axes are to be labelled with x and y, this should be done on the t-SNE widget level.

@pavlin-policar pavlin-policar merged commit 66e52c5 into biolab:master Nov 23, 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