PythonPackage class by citibeth · Pull Request #2778 · spack/spack

citibeth · 2017-01-08T19:16:31Z

Replaces #2709

citibeth · 2017-01-08T19:21:08Z

@adamjstewart This should be the newly rebased PythonPackage PR. Please look in the commit history, I created this PR in three steps:

Got rid of nolink from Python packages (in your branch)
Merge branch '_pythonpackage0' into _develop, taking YOUR branch by default whenever there were conflicts.
Maually reviewed changes from (2), updating when needed. Please pay ESPECIAL attention this commit, these are things I though the original PR got wrong. Of course I might be wrong... so please double-check: d7b9626

citibeth · 2017-01-08T19:39:31Z

@adamjstewart Can I hand this PR back to you now? spack test succeeded on my local machine. And the failed Travis test seems to do with SVN. Really weird... Anyway, I have rebased as I promised, so I think it's time to hand it back.

adamjstewart · 2017-01-08T19:54:27Z

var/spack/repos/builtin/packages/py-csvkit/package.py

    depends_on('py-dbf', type=('build', 'run'))
    depends_on('py-xlrd', type=('build', 'run'))
-    depends_on('py-sqlalchemy', type=('build', 'run'))
+    depends_on('py-SQLAlchemy', type=('build', 'run'))


This change doesn't belong here.

Oops... that needs to be fixed (thought I fixed it). You are right, it should be py-sqlalchemy.

adamjstewart · 2017-01-08T19:56:13Z

var/spack/repos/builtin/packages/py-pexpect/package.py

-
-    extends('python')
-
-    depends_on('py-ptyprocess')


This dep shouldn't be removed.

adamjstewart · 2017-01-08T19:57:37Z

var/spack/repos/builtin/packages/py-pylint/package.py

-    extends('python', ignore=r'bin/pytest')
-    depends_on('py-six', type=('build', 'run'))
-    depends_on('py-astroid', type=('build', 'run'))
-    depends_on('py-logilab-common', type=('build', 'run'))


These deps shouldn't be removed.

adamjstewart · 2017-01-08T19:58:08Z

var/spack/repos/builtin/packages/py-rpy2/package.py

    version('2.5.6', 'a36e758b633ce6aec6a5f450bfee980f')

-    extends('python')
-    depends_on('py-six', type=('build', 'run'))


This dep shouldn't be removed.

adamjstewart · 2017-01-08T20:06:57Z

var/spack/repos/builtin/packages/py-rtree/package.py

+        os.environ['SPATIALINDEX_LIBRARY'] = \
+            join_path(lib, 'libspatialindex.%s' % dso_suffix)
+        os.environ['SPATIALINDEX_C_LIBRARY'] = \
+            join_path(lib, 'libspatialindex_c.%s' % dso_suffix)


This looks like a step in the wrong direction.

citibeth · 2017-01-08T20:43:51Z

@adamjstewart Can you apply the changes that are needed? I intentionally put this PR on the main LLNL repo to facilitate that process.

My only comment would be that I think (but am not 100% sure) that your changes to py-rtree might break it. That's why I changed it back. The env vars need to be set for build, not run. py-rtree then embeds the needed information into its build, RPATH-style.

adamjstewart · 2017-01-08T21:29:27Z

My only comment would be that I think (but am not 100% sure) that your changes to py-rtree might break it. That's why I changed it back. The env vars need to be set for build, not run. py-rtree then embeds the needed information into its build, RPATH-style.

That's what it does. spack_env is the environment Spack sets when you build it, run_env is the environment that gets set in the resulting module file.

citibeth · 2017-01-08T22:10:46Z

I see. I knew that setup_environment() set up the env for modules. But apparently it also sets the environment for building. http://spack.readthedocs.io/en/latest/module_file_support.html?highlight=setup_environment

@tgamblin @alalazo What is the best approach here if we want an environment set for building, but not for running?

adamjstewart · 2017-01-08T22:22:46Z

lib/spack/spack/directives.py

+        # if pkg.extendees:
+        #     directive = 'extends'
+        #     msg = 'Packages can extend at most one other package.'
+        #     raise DirectiveError(directive, msg)


Is this safe to remove? It causes problems for packages that require:

extends('python', ignore='bin/.*')

since there is already:

extends('python')

in the base class.

adamjstewart · 2017-01-08T22:23:10Z

lib/spack/spack/directives.py

-            raise DirectiveError(msg.format(pkg.name, name))
+            directive = 'variant'
+            msg = "Invalid variant name in {0}: '{1}'"
+            raise DirectiveError(directive, msg.format(pkg.name, name))


These calls to DirectiveError were missing a directive.

adamjstewart · 2017-01-08T22:31:54Z

Ok, the rebase is complete now. Let me just port all of the packages added in #2320. I also noticed that none of these packages have the right deptypes (the default instead of build/run) so I'll have to fix those up.

citibeth · 2017-01-08T22:54:57Z

Thanks!

…

On Sun, Jan 8, 2017 at 5:31 PM, Adam J. Stewart ***@***.***> wrote: Ok, the rebase is complete now. Let me just port all of the packages added in #2320 <#2320>. I also noticed that none of these packages have the right deptypes (the default instead of build/run) so I'll have to fix those up. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2778 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd5XWbE0YrfwFwE7PNivoJUUo3K3oks5rQWPbgaJpZM4LdxP6> .

adamjstewart · 2017-01-08T23:21:15Z

Ok, this is probably all of the changes I want to make in this PR. Now the only remaining thing to do is to test it. Does anyone want to see any specific tests before this is ready to merge? I was going to make sure the "important" packages still build (numpy, scipy, matplotlib) and test all of the "weird" packages (everything with an install method that doesn't look like the following):

def install(self, spec, prefix):
    setup_py('install', '--prefix={0}'.format(prefix))

citibeth · 2017-01-09T02:12:32Z

If you want to keep the change to `py-rtree`, it would be good to do the "import" test on it too.

…

On Sun, Jan 8, 2017 at 6:21 PM, Adam J. Stewart ***@***.***> wrote: Ok, this is probably all of the changes I want to make in this PR. Now the only remaining thing to do is to test it. Does anyone want to see any specific tests before this is ready to merge? I was going to make sure the "important" packages still build (numpy, scipy, matplotlib) and test all of the "weird" packages (everything with an install method that doesn't look like the following): def install(self, spec, prefix): setup_py('install', '--prefix={0}'.format(prefix)) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2778 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd-wSW1ripjIFzEPaGpfCY7Cx5Fnfks5rQW9sgaJpZM4LdxP6> .

adamjstewart · 2017-01-09T04:18:15Z

@citibeth Will do. Is it just import rtree?

Also, I'll try to activate all of the packages I build too just to make sure that functionality still works.

citibeth · 2017-01-09T11:56:12Z

Try: from rtree import index I still think the change doesn't feel right. It is akin to putting RPATH in your binary but then overriding it with LD_LIBRARY_PATH anyway. I do not want any extra and unnecessary env vars in the py-rtree module. On Jan 8, 2017 11:18 PM, "Adam J. Stewart" <[email protected]> wrote: @citibeth <https://github.com/citibeth> Will do. Is it just import rtree? Also, I'll try to activate all of the packages I build too just to make sure that functionality still works. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2778 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd0J5rOy_70drNGQxEEe1uugLxxIwks5rQbUIgaJpZM4LdxP6> .

adamjstewart · 2017-01-09T22:09:20Z

I still think the change doesn't feel right. It is akin to putting RPATH in your binary but then overriding it with LD_LIBRARY_PATH anyway. I do not want any extra and unnecessary env vars in the py-rtree module.

I didn't change that. That's how it was to begin with. But I'll test it anyway just to be sure.

citibeth · 2017-01-09T22:15:33Z

On Mon, Jan 9, 2017 at 5:09 PM, Adam J. Stewart ***@***.***> wrote: I still think the change doesn't feel right. It is akin to putting RPATH in your binary but then overriding it with LD_LIBRARY_PATH anyway. I do not want any extra and unnecessary env vars in the py-rtree module.

On third thought, I think my concern is unwarrnated. The new code is: ``` def setup_environment(self, spack_env, run_env): lib = self.spec['libspatialindex'].prefix.lib spack_env.set('SPATIALINDEX_LIBRARY', join_path(lib, 'libspatialindex.%s' % dso_suffix)) spack_env.set('SPATIALINDEX_C_LIBRARY', join_path(lib, 'libspatialindex_c.%s' % dso_suffix)) ``` Am I right that this will set the environment used to build (spack_env) but not the environment in the module files (run_env)? In that case, there should be no problem here. I do appreciate testing, since this is still textually quite different from what was therebefore: https://github.com/LLNL/spack/blob/develop/var/spack/repos/builtin/packages/py-rtree/package.py

adamjstewart · 2017-01-09T22:16:49Z

That's exactly right.

I decided to try installing and activating every single Python package in Spack. That ought to catch a slew of conflicting files. Once I can get everything installed, we'll be safe to merge.

citibeth · 2017-01-09T22:19:13Z

Wow!

…

On Jan 9, 2017 5:16 PM, "Adam J. Stewart" ***@***.***> wrote: That's exactly right. I decided to try installing and activating every single Python package in Spack. That ought to catch a slew of conflicting files. Once I can get everything installed, we'll be safe to merge. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2778 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd9Gv0VyzxvixADCPsoS7A0N4uI4Dks5rQrHRgaJpZM4LdxP6> .

adamjstewart · 2017-01-11T20:59:42Z

Ok, as of the last commit, I am able to install every py-* package except:

py-genders: missing a genders dep
py-meep: header file linking broke, not sure why. It was complicated to begin with
py-mysqldb1: missing dependency on mysql
py-nbconvert: failed to download CSS?
py-pygobject: tarball disappeared, can't find another version on PyPi
py-pygtk: invalid spec due to cairo+X vs cairo~X
py-rpy2: missing dependencies on icuuc, icui18

I added FIXME comments to these packages. Someone can fix them as they need them.

Ok, last step is to try activating all of these packages. We'll see what conflicts we find!

citibeth · 2017-01-12T17:30:37Z

var/spack/repos/builtin/packages/py-flake8/package.py

+    # These dependencies breaks concretization
+    # See https://github.com/LLNL/spack/issues/2793
+    # depends_on('py-configparser', when='^python@:3.3.999', type=('build', 'run'))  # noqa
+    # depends_on('py-enum34', when='^python@:3.1.999', type=('build', 'run'))


I believe you can use [email protected] here; [email protected] acts like a set that also matches [email protected]. Not sure where else it happen in this PR (and this one's in a comment); but something to look out for.

Are you able to grep through and fix for places where .999 occurs?

I don't believe that's the case. You need to use a range. 3.3 only matches 3.3, so if you tried to install something that depends on [email protected], it will try to fetch python-3.3.tar.gz, which won't exist.

citibeth · 2017-01-12T17:31:14Z

var/spack/repos/builtin/packages/py-ipykernel/package.py


-    extends('python')
-
    depends_on('[email protected]:2.7.999,3.3:')


Should be depends_on('[email protected],3.3:').

This won't work. There is no Python version 2.7, so it will never be matched. Try editing it and running spack spec.

citibeth · 2017-01-12T17:37:08Z

@adamjstewart A BIG THANKS! This is the kind of detail work that moves a project from "sort of works" to "really usable."

I agree, secret downloads by pip are a problem, and they should never be happening (or allowed). It would be nice if there's a way to regression-test this property. My thoughts are either we get a regression test in an Internet-free container somewhere; or we find a way to disable PIP to catch these errors. Either way, fixing the existing PIP problems as you have done is a big step forward.

citibeth · 2017-01-12T17:52:53Z

Wouldn't this work? I saw the code to do exactly that in `version.py`. depends_on('py-enum34', when='^python@:3.1', type=('build', 'run'))

…

On Thu, Jan 12, 2017 at 12:38 PM, Adam J. Stewart ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In var/spack/repos/builtin/packages/py-flake8/package.py: > @@ -57,11 +57,12 @@ class PyFlake8(Package): # mccabe >= 0.2.1, < 0.5 ***@***.***:0.4.0', ***@***.***', type=('build', 'run')) - depends_on('py-configparser', when='^python@:3.3.999', type=('build', 'run')) - depends_on('py-enum34', when='^python@:3.1.999', type=('build', 'run')) + # These dependencies breaks concretization + # See #2793 + # depends_on('py-configparser', when='^python@:3.3.999', type=('build', 'run')) # noqa + # depends_on('py-enum34', when='^python@:3.1.999', type=('build', 'run')) I don't believe that's the case. You need to use a range. 3.3 only matches 3.3, so if you tried to install something that depends on ***@***.***, it will try to fetch python-3.3.tar.gz, which won't exist. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2778>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd_bGiAmTUELubYqVBIpEUNPEz2ZSks5rRmT7gaJpZM4LdxP6> .

citibeth · 2017-01-12T17:53:19Z

Would this work? depends_on('[email protected]:2.7,3.3:')

…

On Thu, Jan 12, 2017 at 12:41 PM, Adam J. Stewart ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In var/spack/repos/builtin/packages/py-ipykernel/package.py: > @@ -42,15 +42,10 @@ class PyIpykernel(Package): version('4.1.1', '51376850c46fb006e1f8d1cd353507c5') version('4.1.0', '638a43e4f8a15872f749090c3f0827b6') - extends('python') - ***@***.***:2.7.999,3.3:') This won't work. There is no Python version 2.7, so it will never be matched. Try editing it and running spack spec. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2778>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd59rU06xWXKyhfkBXAD4aSYYg6V9ks5rRmXFgaJpZM4LdxP6> .

adamjstewart · 2017-01-12T17:56:06Z

depends_on('py-enum34', when='^python@:3.1', type=('build', 'run'))

This is valid, although it doesn't solve the problem reported in #2793.

depends_on('[email protected]:2.7,3.3:')

This does not work, I've tried it before. It complains about a range that starts and stops at the same version.

citibeth · 2017-01-12T18:04:09Z

Arrgh. It sounds like the version range stuff is still a WIP. Thanks again for all this detail work.

…

On Thu, Jan 12, 2017 at 12:56 PM, Adam J. Stewart ***@***.***> wrote: depends_on('py-enum34', when='^python@:3.1', type=('build', 'run')) This is valid, although it doesn't solve the problem reported in #2793 <#2793>. ***@***.***:2.7,3.3:') This does not work, I've tried it before. It complains about a range that starts and stops at the same version. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2778 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd2gNykJXbx9dUbiqCR8Tv1QnwZj-ks5rRmk4gaJpZM4LdxP6> .

citibeth · 2017-01-12T18:05:31Z

GitHub doesn't allow me to approve this PR because I'm its "author." But yes, I do approve it! 👍

adamjstewart · 2017-01-12T18:14:26Z

Haha, I "approved" it for you

adamjstewart · 2017-01-12T18:14:47Z

I wish there was an easier way to reassign ownership.

citibeth · 2017-01-13T17:52:42Z

@tgamblin I recommend this gets merged ASAP. It's a BIG change. And without a merge, new Python packages continue to come in that will have to be fixed after-the-fact.

adamjstewart · 2017-01-15T13:39:41Z

I tried rebasing this to convert the latest new Python packages, figuring it would be easy since there are no conflicts, but it didn't work. @citibeth I'm not sure how you went about rebasing this, but I don't think it can be rebased easily anymore. It still asks me to resolve conflicts for things like the nolink conversion that I thought you had worked out. @tgamblin This needs to be merged or else I'm no longer going to support it. It has become too difficult to maintain. Basically, if there ever becomes any conflicts with existing files, I can't do anything about it anymore.

citibeth · 2017-01-16T17:56:29Z

I rebased the last one using black magic and ill-defined procedures. I will take a look at it...

…

On Sun, Jan 15, 2017 at 8:39 AM, Adam J. Stewart ***@***.***> wrote: I tried rebasing this to convert the latest new Python packages, figuring it would be easy since there are no conflicts, but it didn't work. @citibeth <https://github.com/citibeth> I'm not sure how you went about rebasing this, but I don't think it can be rebased easily anymore. It still asks me to resolve conflicts for things like the nolink conversion that I thought you had worked out. @tgamblin <https://github.com/tgamblin> This needs to be merged or else I'm no longer going to support it. It has become too difficult to maintain. Basically, if there ever becomes any conflicts with existing files, I can't do anything about it anymore. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2778 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd3sn7kEhpdgU8oitPoa28vQ9ta9mks5rSiGegaJpZM4LdxP6> .

adamjstewart · 2017-01-16T18:38:18Z

No need to take a look. As long as we can get this merged soon, it shouldn't be a problem, I hope.

- Add a PythonPackage class with build system support. - Support build phases in PythonPackage - Add a custom sanity check for PythonPackages - Get rid of nolink dependencies in python packages - Update spack create to use new PythonPackage class - Port most of Python packages to new PythonPackage class - Conducted a massive install and activate of Python packages. - Fixed bugs introduced by install and activate. - Update API docs on PythonPackage

tgamblin · 2017-01-17T02:23:50Z

@adamjstewart @citibeth this wasn't so much rebased as merged. It has two bases at different points in the history of develop, which were merged together then added to.

In the future, if you combine two branches like this, it's probably best to rebase one onto develop, then rebase the other onto that if you hope to continue rebasing on develop as things progress. This keeps this history linear. Turning on rerere makes the process much less painful.

OTOH, you can also just keep merging develop into your branch instead of bothering to rebase. Since we squash anyway, continuously merging develop and continuously rebasing on develop will end up looking the same in the end.

It looks like most of this work is @adamjstewart's, but if I squash with GitHub, the commit shows up as authored by the PR submitter. So, I just squashed the result of the merge manually and force-pushed a single commit by @adamjstewart.

@adamjstewart: Can you review these changes and make sure they work and look ok? There was only one minor conflict in spack create that needed to be resolved. Given that you've added a lot of stability with this stuff, I'd like to get this into 0.10.

tgamblin · 2017-01-17T02:25:21Z

@adamjstewart: note that you should still be able to push commits on top of this if you need to change anything.

citibeth · 2017-01-17T02:33:25Z

OTOH, you can also just keep merging develop into your branch instead of bothering to rebase. Since we squash anyway, continuously merging develop and continuously rebasing on develop will end up looking the same in the end.

I think that's the best approach here.

It looks like most of this work is @adamjstewart <https://github.com/adamjstewart>'s, but if I squash with GitHub, the commit shows up as authored by the PR submitter. So, I just squashed the result of the merge manually and force-pushed a single commit by @adamjstewart <https://github.com/adamjstewart>.

I agree, @adamjstewart should get the credit.

adamjstewart · 2017-01-17T13:50:38Z

Everything looks good to me! Thanks for getting this in @tgamblin. And thanks to @citibeth for helping get this rebased. Even if it was hacky, it certainly saved me a lot of time.

Now I'll have to start thinking about the next steps for the PythonPackage class. I want to add import tests to make sure the module can actually be imported after installation. Any requests aside from that?

citibeth · 2017-01-17T15:24:52Z

Any requests aside from that?

@adamjstewart While #2778 was being prepared, a few packages that SHOULD use PythonPackage but didn't slipped through. Probably best to chase them down and convert them to PythonPackage now too (it's only a handful, if I recall). Then we can set the standard that Python packages use PythonPackage, with a few exceptions of course.

I want to add import tests to make sure the module can actually be

imported after installation. Good idea.

…

On Tue, Jan 17, 2017 at 8:50 AM, Adam J. Stewart ***@***.***> wrote: Everything looks good to me! Thanks for getting this in @tgamblin <https://github.com/tgamblin>. And thanks to @citibeth <https://github.com/citibeth> for helping get this rebased. Even if it was hacky, it certainly saved me a lot of time. Now I'll have to start thinking about the next steps for the PythonPackage class. I want to add import tests to make sure the module can actually be imported after installation. Any requests aside from that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2778 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd2a1vQvwqpsmTZ69pGpOIBz7_HB5ks5rTMcwgaJpZM4LdxP6> .

citibeth added python WIP labels Jan 8, 2017

citibeth mentioned this pull request Jan 8, 2017

[WIP] Preliminary PythonPackage class #2709

Closed

adamjstewart reviewed Jan 8, 2017

View reviewed changes

adamjstewart changed the title ~~Preliminary PythonPackage class (2)~~ [WIP] Preliminary PythonPackage class Jan 8, 2017

adamjstewart reviewed Jan 8, 2017

View reviewed changes

adamjstewart mentioned this pull request Jan 9, 2017

documentation: build-system phases + build-time tests #2780

Merged

2 tasks

citibeth mentioned this pull request Jan 10, 2017

py-git2: New Package #1971

Merged

alalazo mentioned this pull request Jan 10, 2017

Fix spack load --dependencies #1662

Closed

adamjstewart removed the WIP label Jan 12, 2017

citibeth commented Jan 12, 2017

View reviewed changes

citibeth assigned adamjstewart Jan 12, 2017

adamjstewart approved these changes Jan 12, 2017

View reviewed changes

tgamblin changed the title ~~Preliminary PythonPackage class~~ PythonPackage class Jan 17, 2017

tgamblin force-pushed the features/pythonpackage2 branch from fd61ae1 to 25869a0 Compare January 17, 2017 02:22

tgamblin merged commit c0aaa8f into develop Jan 17, 2017

This was referenced Jan 17, 2017

Allow packages to extend multiple other packages #987

Open

building py-yt now fails with cannot depend twice error #2728

Closed

adamjstewart deleted the features/pythonpackage2 branch January 20, 2017 15:03

adamjstewart mentioned this pull request Feb 9, 2017

fixes requires for pygtk #3077

Merged

adamjstewart mentioned this pull request Mar 29, 2017

Allow py-py2cairo to be built with cairo~X #3620

Merged


		extends('python')

		depends_on('[email protected]:2.7.999,3.3:')

Conversation

citibeth commented Jan 8, 2017

Uh oh!

citibeth commented Jan 8, 2017

Uh oh!

citibeth commented Jan 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

citibeth commented Jan 8, 2017

Uh oh!

adamjstewart commented Jan 8, 2017

Uh oh!

citibeth commented Jan 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamjstewart commented Jan 8, 2017

Uh oh!

citibeth commented Jan 8, 2017 via email

Uh oh!

adamjstewart commented Jan 8, 2017

Uh oh!

citibeth commented Jan 9, 2017 via email

Uh oh!

adamjstewart commented Jan 9, 2017

Uh oh!

citibeth commented Jan 9, 2017 via email

Uh oh!

adamjstewart commented Jan 9, 2017

Uh oh!

citibeth commented Jan 9, 2017 via email

Uh oh!

adamjstewart commented Jan 9, 2017

Uh oh!

citibeth commented Jan 9, 2017 via email

Uh oh!

adamjstewart commented Jan 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

citibeth commented Jan 12, 2017

Uh oh!

citibeth commented Jan 12, 2017 via email

Uh oh!

citibeth commented Jan 12, 2017 via email

Uh oh!

adamjstewart commented Jan 12, 2017

Uh oh!

citibeth commented Jan 12, 2017 via email

Uh oh!

citibeth commented Jan 12, 2017

Uh oh!

adamjstewart commented Jan 12, 2017

Uh oh!

adamjstewart commented Jan 12, 2017

Uh oh!

citibeth commented Jan 13, 2017

Uh oh!

adamjstewart commented Jan 15, 2017

Uh oh!

adamjstewart commented Jan 11, 2017 •

edited

Loading