Add plotting option to Fingerprint plot #787
Conversation
There was a problem hiding this comment.
This looks good @theref, could you throw some of the images here to take a look?
Also, as the defaults are True (which I like) could you update the images for the docs?
| @@ -267,7 +267,7 @@ def fingerprint(self, turns=50, repetitions=10, step=0.01, processes=None, | |||
| self.data = self.generate_data(self.interactions, self.points, edges) | |||
| return self.data | |||
|
|
|||
| def plot(self, col_map='seismic', interpolation='none', title=None): | |||
| def plot(self, col_map='seismic', interpolation='none', title=None, colorbar=True, labels=True): | |||
There was a problem hiding this comment.
Surely this has to be colourbar? 😉
There was a problem hiding this comment.
(I know it's called colorbar in matplotlib, but still.....)
There was a problem hiding this comment.
To be clear - I'm not actually asking for any change here.
| @@ -284,6 +284,8 @@ def plot(self, col_map='seismic', interpolation='none', title=None): | |||
| http://matplotlib.org/examples/images_contours_and_fields/interpolation_methods.html | |||
| title : str, optional | |||
| A title for the plot | |||
| colorbar : bool, optional | |||
| Choose whether the colorbar should be included or not | |||
There was a problem hiding this comment.
labels needs to be added here.
There was a problem hiding this comment.
also col_map and interpolation
There was a problem hiding this comment.
col_map and interpolation are already in there
There was a problem hiding this comment.
agghh. sorry - that was github folding lines on me and I hadn't noticed
There was a problem hiding this comment.
filename appears in the docstring but doesn't seem to be in the signature. Is that still required?
| plt.xticks([0, len(plotting_data) - 1], ['0', '1']) | ||
| plt.yticks([0, len(plotting_data) - 1], ['0', '1']) | ||
|
|
||
| if not labels: |
There was a problem hiding this comment.
Can you add a test with labels=False and colorbar=False (just to check the plot runs).
Not sure if you saw that comment @theref |
| ticks = [min_score, (max_score + min_score) / 2, max_score] | ||
| fig.colorbar(cax, ticks=ticks) | ||
|
|
||
| plt.xlabel('y') |
There was a problem hiding this comment.
xlabel is 'y' and ylabel is 'x'?
There was a problem hiding this comment.
yeah, it's due to the way numpy shapes the data:
plotting_data = np.reshape(ordered_data, (size, size))
@drvinceknight do you think this is going to cause confusion? should I work on getting the plots rotated?
There was a problem hiding this comment.
Yeah, I think it's worth the change.
|
Just the one request for clarification from me. |
|
A couple of examples:
|
|
This looks good, need to fix the conflicts with master and can you change |
| @@ -284,6 +284,8 @@ def plot(self, col_map='seismic', interpolation='none', title=None): | |||
| http://matplotlib.org/examples/images_contours_and_fields/interpolation_methods.html | |||
| title : str, optional | |||
| A title for the plot | |||
| colorbar : bool, optional | |||
| Choose whether the colorbar should be included or not | |||
There was a problem hiding this comment.
also col_map and interpolation
| plt.xlabel('$x$') | ||
| plt.ylabel('$y$', rotation=0) | ||
| # ax.xaxis.tick_top() | ||
| # ax.xaxis.set_label_position('top') |
There was a problem hiding this comment.
If these two lines aren't required, can you get rid of them, please?
| plt.ylabel('$y$', rotation=0) | ||
| # ax.xaxis.tick_top() | ||
| # ax.xaxis.set_label_position('top') | ||
| ax.tick_params(axis=u'both', which=u'both', length=0) |
There was a problem hiding this comment.
There's no need for 'u' now as we no longer support Python 2
Shows that `colorbar` and 'labels' can be set to `False` to produce a plot with no extras. Also updates the plots in the documentation to show both options
|
I'm happy. @meatballs I suggest we wait till @marcharper wakes to merge, as all 3 of us have commented here now :) |
|
@marcharper note that the examples in the conversation above are no longer correct, look at the files changed to see what they look like now :) |
Adds: