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

rework plotting to allow for plot_axes #79

Merged
merged 7 commits into from
Aug 12, 2020

Conversation

sroet
Copy link
Collaborator

@sroet sroet commented Jul 13, 2020

Closes #78

This reworks the plotting of ContactCount, it split out the axes plotting code in ContactCount.plot_axes()

This also allows users to use plot_axes to allow for greater flexibility (like with subplots)

This now works:

import contact_map as cm
import mdtraj as md
import matplotlib.pyplot as plt

traj = md.load("contact_map/tests/trajectory.pdb")
contactmap = cm.ContactFrequency(traj)
# Old style
fig1, ax1 = contactmap.atom_contacts.plot(figsize=(12,12))

# New posibility
fig, axs = plt.subplots(2,2, figsize=(12,12))
axs = axs.flatten()
for ax in axs:
    _ = contactmap.atom_contacts.plot_axes(ax=ax)

BIG DIFFERENCE between plot() and plot_axes(): No colorbar for plot_axes()

Colorbars are hard for subplots in matplotlib (part of the figure, but also plotted with the last ax of subplots)
An user can add a seperate colorbar to the figure if they want, using any of the answers here.

Edit: current default plots a colorbar in every axes, can be turned off with_colorbar=False

[API-BREAK] - in .plot_utils the call-signature of ranged_colorbar has changed:
- unused kwarg name is removed
- new kwarg ax is added, this is the axes that is put into plt.colorbar( ax=ax)

@nffaruk Would you be able to see if this solves your issue?
You can install this version of the code by running
pip install git+https://github.com/sroet/contact_map.git@allow_fig_ax_kw
preferably in a new conda environment (or you might need to uninstall first, depending on how you installed Contact Map Explorer in the first place)

@dwhswenson
Copy link
Owner

dwhswenson commented Jul 13, 2020

Both of us putting thought into this today! Posts with minutes of each other. 😆

For colorbars, try passing ax down through contact_counter._colorbar and to ranged_colorbar. Then something like:

if ax is None:
    colorbar = plt.colorbar
else:
    # might actually need a partial here, if plt.colorbar doesn't have ax/cax params
    colorbar = ax.figure.colorbar
cb = colorbar(sm, fraction=0.046, pad=0.04, ax=ax)  # maybe cax=ax?

See docs on Figure.colorbar for details on ax and cax kwargs.

@dwhswenson
Copy link
Owner

The idea above should act as I suggested in #78: defaults to adding a colorbar for each axis, although the user can override with with_colorbar=False. Then the next step would be to make a more accessible version of colorbar. I think the signature for that would be something like:

def colorbar(cmap, is_divergent, cbar_min, ax=None):
    ...

cmap could be string (plt.get_cmap to get the func) or an actual cmap function. is_divergent is a bool, and would use vmin, vmax = -1.0, 1.0 if True, 0.0, 1.0 if False. cbar_min is 0 or -1. If you look at contact_counter._colorbar, the main thing it does is use np.floor to map -1.0 ≤ x < 0.0 to cbmin = -1 and 0.0 ≤ x < 1.0 to cbmin = 0.

I'll need to write some docs at some point to explain how to use divergent vs. sequential color maps, and why I think there's so much virtue in using the top half of a divergent color map. But this is definitely a more user-friendly way to build a colorbar.

The point of all of this is to prep info that gets fed into ranged_colorbar, so it can actually create the colorbars. With this, passing the ax with multiple axes (after using with_axes=False in the original call to plot_axes()) should let you automatically create a single colorbar for the whole figure. (again, see the docs on ax in Figure.colorbar.)

I also lean toward the overall idea of gathering some of the matplotlib stuff together at some point. In principle, the future might involve non-matplotlib plotting (e.g., interactive Bokeh that tells you more about the contact when you hover). Feel free to move things around if it makes sense to you.

@nffaruk
Copy link
Contributor

nffaruk commented Jul 13, 2020

@nffaruk Would you be able to see if this solves your issue?
You can install this version of the code by running
pip install git+https://github.com/sroet/contact_map.git@allow_fig_ax_kw

Thanks both of you for looking into this in detail, and so quickly too! Unfortunately, I'm unable to install that version, I'm still on Python 2.7.16 (for compatibility with my lab's software) and I get this error:

Collecting git+https://github.com/sroet/contact_map.git@allow_fig_ax_kw
  Cloning https://github.com/sroet/contact_map.git (to revision allow_fig_ax_kw) to /tmp/pip-req-build-Xt1kBk
  Running command git clone -q https://github.com/sroet/contact_map.git /tmp/pip-req-build-Xt1kBk
  Running command git checkout -b allow_fig_ax_kw --track origin/allow_fig_ax_kw
  Switched to a new branch 'allow_fig_ax_kw'
  Branch allow_fig_ax_kw set up to track remote branch allow_fig_ax_kw from origin.
    ERROR: Command errored out with exit status 1:
     command: /home/nffaruk/.conda/envs/upside/bin/python -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-req-build-Xt1kBk/setup.py'"'"'; __file__='"'"'/tmp/pip-req-build-Xt1kBk/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-1Jci8s
         cwd: /tmp/pip-req-build-Xt1kBk/
    Complete output (12 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-req-build-Xt1kBk/setup.py", line 126, in <module>
        write_installed_version_py()
      File "/tmp/pip-req-build-Xt1kBk/setup.py", line 99, in write_installed_version_py
        version_finder = VersionPyFinder()
      File "/tmp/pip-req-build-Xt1kBk/setup.py", line 29, in __init__
        self.functions = self._get_functions(self.filename)
      File "/tmp/pip-req-build-Xt1kBk/setup.py", line 93, in _get_functions
        exec(compile(tree, filename="version.py", mode='exec'), locs)
      File "version.py", line 12, in <module>
    TypeError: exceptions must be old-style classes or derived from BaseException, not NoneType
    ----------------------------------------
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.

@nffaruk
Copy link
Contributor

nffaruk commented Jul 13, 2020

Also, regarding #78 @dwhswenson 's option 3 with the kwargs, according to this, it doesn't seem like there would be an issue mixing explicitly named/default kwargs with the arbitrary **kwargs unpacking?

@sroet
Copy link
Collaborator Author

sroet commented Jul 13, 2020

@nffaruk Unfortunately, Python 2 has reached end-of-life on 01-01-2020.

I see you are using conda, is there a particular reason why you can't separate the conda environments for your lab software and contact map explorer?

If not, you can try the following:

conda create -n contact_map python=3
conda activate contact_map 
conda install --only-deps contact_map
pip install git+https://github.com/sroet/contact_map.git@allow_fig_ax_kw

This will make a new conda environment for contact map, if you want to activate it in a new shell you can use conda activate contact_map

Also, regarding #78 @dwhswenson 's option 3 with the kwargs, according to this, it doesn't seem like there would be an issue mixing explicitly named/default kwargs with the arbitrary **kwargs unpacking?

The problem @dwhswenson indicated is that not all valid **kwargs for matplotlib.subplots() lead to error free plotting code on our side (i.example if you give nrows=2 or any other way that we generate more than 1 axes, it fails, as we don't know which axis to plot in).

@dwhswenson
Copy link
Owner

@sroet : This particular problem can be fixed by updating the vendored copy of setup.py. I had to fix this problem for OPS a few months back. Diff: dwhswenson/autorelease@4e74069.

I don't think we've added anything that breaks Py2 compatibility yet, but that will definitely come soon.

@sroet
Copy link
Collaborator Author

sroet commented Jul 13, 2020

@dwhswenson any preference on the codeclimate issue? (Function ranged_colorbar has 6 arguments (exceeds 5 allowed)

I opted to remove the name kwarg, as it was unused/hard-coded anyway

@dwhswenson
Copy link
Owner

@dwhswenson any preference on the codeclimate issue? (Function ranged_colorbar has 6 arguments (exceeds 5 allowed)

I opted to remove the name kwarg, as it was unused/hard-coded anyway

That's fine. Or I'll ignore it, if codeclimate keeps complaining. It is designed to be a function that I can copy into other projects in the future. It does a lot more than what we use.

@sroet
Copy link
Collaborator Author

sroet commented Jul 13, 2020

@sroet : This particular problem can be fixed by updating the vendored copy of setup.py. I had to fix this problem for OPS a few months back. Diff: dwhswenson/autorelease@4e74069.

I don't think we've added anything that breaks Py2 compatibility yet, but that will definitely come soon.

awesome, applied the same code. @nffaruk do you mind rerunning the install?

@sroet
Copy link
Collaborator Author

sroet commented Jul 13, 2020

@dwhswenson this is ready for a review (apparently our current test-suite catches all changes made to the code in this PR)

@nffaruk
Copy link
Contributor

nffaruk commented Jul 13, 2020

Got it to install and it works great, thanks! For sure developers shouldn't have the burden of supporting end of life software and it's straightforward to switch environments, but I was hoping there would be an easy fix considering our PI is a big user of your package (this subplot request came from him) and uses it in conjunction with a bunch of analysis functions from our side. But we will have to get on the inescapable migration to Python 3 soon.

Copy link
Owner

@dwhswenson dwhswenson left a comment

Choose a reason for hiding this comment

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

Small typo in a docstring, otherwise LGTM. I noticed that the colorbars don't go the full height of the plot (compare output of cell 12 and cell 11 in the notebook), but that can be dealt with later.

One other thing: please go ahead and bump the version to 0.6.0.dev0 (in setup.cfg and ci/conda-recipe/meta.yaml). This will definitely be part of a 0.6.0 release, not a 0.5.1 release (if I can fix the Travis/autorelease issues for 0.5.1).

@@ -20,8 +21,8 @@ def ranged_colorbar(cmap, norm, cbmin, cbmax, name="Partial Map"):
minimum value for the colorbar
cbmax : float
maximum value for the colorbar
name : str
name for the submap to be created
ax : matplotlib.axes
Copy link
Owner

Choose a reason for hiding this comment

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

capitalize: matplotlib.Axes

@sroet
Copy link
Collaborator Author

sroet commented Jul 14, 2020

I noticed that the colorbars don't go the full height of the plot

Yeah, I think this is a difference between "taking space from an existing axes" and "adding a colorbar axes", but not to sure about that. Give me an hour to test something out

@dwhswenson
Copy link
Owner

No rush... I probably won't merge this until after 0.5.1 gets released, anyway. I have a few ideas to test that might fix the release process.

@sroet
Copy link
Collaborator Author

sroet commented Jul 14, 2020

Figured it out: it is due to the changed aspect ratio of the axes (default is 10,8) but for that plot I used (20,6). If I change it to (30,8) it plots as intended again.

@sroet
Copy link
Collaborator Author

sroet commented Jul 14, 2020

All set ;)

@dwhswenson
Copy link
Owner

With 0.5.1 out, I'll go ahead and merge this as the first step to 0.6.0!

@dwhswenson dwhswenson merged commit e4ebff6 into dwhswenson:master Aug 12, 2020
@dwhswenson dwhswenson mentioned this pull request Aug 12, 2020
@sroet sroet deleted the allow_fig_ax_kw branch August 17, 2020 07:56
@dwhswenson dwhswenson mentioned this pull request Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subplots for ContactFrequency
3 participants