surmeierlab / neurphys

python package for ephys and calcium imaging IO and analysis
GNU General Public License v3.0
3 stars 2 forks source link

Nuplot wish list #26

Open DGalt opened 7 years ago

DGalt commented 7 years ago

Couple things it would be nice to have in nuplot

  1. Make clean_axis more generally useful by making the units argument optional. A lot of time I draw my own scale bars, so I don't need clean_axis to do it.

  2. For nu_raster provide the option to change the height of the raster bars.

  3. Give nu_raster option of using clean_axis. This one is maybe less necessary if 1 is implemented (although 1 would need to implemented to get this to work anyway).

I might get around to doing these in the next day or two since I've implemented them already in my own stuff, just wanted to open discussion first to get thoughts on changes.

chadestep commented 7 years ago

(Note: deleted previous comment I made under a different account on accident)

I have strong feelings about # 1 and a few questions about the rest.

It's a bit paternalistic to mandate units, but I would still advise against making units optional as it will ensure that proper scale bars are in figure. They can be very easily deleted later on, and forcing users to do something like this should be necessary when removing the rest of the axes as it is another barrier to prevent purposeful (or inadvertent) figure manipulation.

You mean change height so they overlap? If that's not the case and you want higher bars, just make the subplot taller. Am I missing something?

Why not just pass clean_axis on that axis object?

DGalt commented 7 years ago

How do you remove them later? You mean in illustrator? There are lots of cases where having it draw all those extra things screws up the generation of a more complicated figured. Or, like in the scenario that I'm dealing with right now with making a raster and having scale bars is unnecessary due to way the figure is set up, I'm unable to use nu_plot's clean_axis function since I don't want it drawing things anywhere.

Change the height so they don't overlap actually. I find the current heights to be too tall. Changing the height of the axis doesn't fix that.

See above for response about point 3

Edit: I should clarify what I mean above. I can obviously use clean_axis and just pass it some random string and it will work fine. The fact that y_hlines is essentially an optional parameter, though, would seem to imply that y_units should be as well

chadestep commented 7 years ago

I see what you're saying now (haven't looked at the code in quite a while...).

Go ahead and submit the pull request. I had my reasons for doing it the way I did, but it involved adding a bunch of features that I never got around to adding and now I probably never will.

As for the raster, you're having problems with raster lines overlapping? I've never seen that behavior, can you attach an example?

DGalt commented 7 years ago

It's not really that they overlap, just that they run in to each other, which I don't think looks good.

E.g. - this is what the current implementation looks like: too long

and this is with my making them a bit shorter:

shorter

chadestep commented 7 years ago

Oh, okay.

I wouldn't change the vline height as much as you did, but splitting the difference would look good.

DGalt commented 7 years ago

Well, regardless, an easy way to set that would be helfpul. Took me a couple of tries to realize how you were drawing these :P

chadestep commented 7 years ago

Yeah, writing the raster function was actually a pain in the ass. I originally wanted to write it using matplotlib linecollections but the documentation was... not great. Looks like it is better now and would probably be a better way to go. Same with the input data structure, but that's for another day.