ContextLab / hypertools

A Python toolbox for gaining geometric insights into high-dimensional data
http://hypertools.readthedocs.io/en/latest/
MIT License
1.83k stars 160 forks source link

simplifying API #105

Closed jeremymanning closed 7 years ago

jeremymanning commented 7 years ago

The API could be simplified in a few places. For example:

rarredon commented 7 years ago

Hi there, I'm Ryan. I found your project through the Mozilla Sprint. This issue sounds like something I can work on. I have a few questions to clarify.

  1. In regards to the first bullet point, where does the alignment algorithm fit in with normalization and reduction? From details of the other bullet points, it sounds like alignment should come after reduction. So should order of operations be: normalization, reduction, then alignment?

  2. The plot function (hyperplot.plot) has many keyword args. Should I add the align flag at the end, or maybe somewhere in the middle? I think standard practice is to put the more heavily used arguments closer to the front.

andrewheusser commented 7 years ago

Hi Ryan! We'd love to have you help out! Regarding your questions -

In regards to the first bullet point, where does the alignment algorithm fit in with normalization and reduction? From details of the other bullet points, it sounds like alignment should come after reduction. So should order of operations be: normalization, reduction, then alignment?

I believe the order should be normalize -> align -> reduce. @jeremymanning can you verify this order of transforms?

The plot function (hyperplot.plot) has many keyword args. Should I add the align flag at the end, or maybe somewhere in the middle? I think standard practice is to put the more heavily used arguments closer to the front.

I would put that right after the ndims flag (i clustered the 'data transforms' in that section). It should be false by default, and throw an error if the len(x) < 1 (its doesn't make sense to 'align' something to itself). If you could add a short description to the doc string, that would also be helpful. you can find info on the operation in the docstring of hyp.tools.align

Let me know if any questions come up! you can leave comments here, or in our gitter: https://gitter.im/hypertools/Lobby

jeremymanning commented 7 years ago

For plotting, I think we want normalize --> reduce --> align. This is because:

Re: the positions of the new keyword arguments, @andrewheusser's suggestion looks good to me...

rarredon commented 7 years ago

Thanks guys, your comments are helpful.

Also, in regards to bullet point 3, how would you like the matrices to be zero padded? Should they be padded on the left, on the right, on both edges equally (if padding width is even)? Maybe there could be an extra kw arg pad_edge if you think it would be beneficial for the user to choose.

jeremymanning commented 7 years ago

Great question!

None of the functions should pad with zeros, except hyp.tools.align. (And I believe this is already implemented.) So we shouldn't need to add any zero padding to implement this issue. To explain our reasoning for not zero padding (except for align):

The basic assumption we make in reduce is that each dimension (feature) of the input is the same across the different input matrices. So that assumption is violated if the dimension labels don't match across the matrices. As a basic easy-to-implement sanity check, if the sizes of the inputs don't match (along the columns), we know for sure that the column labels can't possibly match, so the reduce function should error out. Additional thoughts on this:

The exception to all of this is align. Align's basic assumption is that the dimension labels don't match. So for align, we should zero pad (and it doesn't actually matter how we do it-- align should give the same answer regardless).

andrewheusser commented 7 years ago

@jeremymanning - re dataframes, we do not do any reordering. If the input data is a dataframe (or list of dfs) we:

  1. transform all text columns into binary dummy variables (see here leaving the numerical columns as they are
  2. turn the whole df into a np array
  3. optionally output the column labels if return_labels=True

I'll add an issue to check to see if the column labels are the same across dfs when reducing. if they are the same, just out of order, they will be resorted.

@rarredon I will be working on hypertools all day today, so ping me if you need clarification as things come up!

rarredon commented 7 years ago

Okay, so I'll finish up the other parts, and forget the part about zero padding. Should be done soon!

jeremymanning commented 7 years ago

That's great @rarredon-- thanks!

jeremymanning commented 7 years ago

Thanks @rarredon! Closing this issue...

jeremymanning commented 7 years ago

Actually...re-opening, sorry. I realized: the tutorials on readthedocs still need updating.

jeremymanning commented 7 years ago

(Should be done only after we push the next version of hypertools to pip)

andrewheusser commented 7 years ago

do you mean adding some examples using these new features to the gallery? @lucywowen is working on some tutorials like http://cdl-quail.readthedocs.io/en/latest/tutorial.html, but those are still underway

jeremymanning commented 7 years ago

No-- I just mean updating the current examples (e.g. align, reduce, etc.) to use the simpler syntax. But the tutorials will be helpful too!

andrewheusser commented 7 years ago

Got it! I updated all the examples and created a log of the changes, ready for pip?

jeremymanning commented 7 years ago

ready for pip-- pip, pip, pooray! 🎉

jeremymanning commented 7 years ago

eek-- wait!!! saving animations is now crashing...

In [8]: hyp.plot([group1, group2], animate=True, zoom=2.5, save_path='/Users/jmanning/Desktop/animation.mp4')
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-8-1eebb1ac5546> in <module>()
----> 1 hyp.plot([group1, group2], animate=True, zoom=2.5, save_path='/Users/jmanning/Desktop/animation.mp4')

/Users/jmanning/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/hypertools/plot/plot.pyc in plot(x, fmt, marker, markers, linestyle, linestyles, color, colors, palette, group, labels, legend, title, elev, azim, ndims, normalize, n_clusters, save_path, animate, duration, tail_duration, rotations, zoom, chemtrails, precog, bullettime, frame_rate, explore, show)
    271     if save_path is not None:
    272         if animate:
--> 273             Writer = animation.writers['ffmpeg']
    274             writer = Writer(fps=frame_rate, bitrate=1800)
    275             line_ani.save(save_path, writer=writer)

NameError: global name 'animation' is not defined
jeremymanning commented 7 years ago

^ this also seems to affect 0.2.0 😿

andrewheusser commented 7 years ago

do you have the most version of master? i patched that

jeremymanning commented 7 years ago

ah-- nvm; just installed directly from git and it works again. sorry for the scare.

andrewheusser commented 7 years ago

all good! pushing to pip!