Skip to content
This repository has been archived by the owner on Feb 9, 2020. It is now read-only.

Using WoodburyMatrices #56

Closed
wants to merge 2 commits into from
Closed

Using WoodburyMatrices #56

wants to merge 2 commits into from

Conversation

sglyon
Copy link
Contributor

@sglyon sglyon commented Mar 14, 2015

These have been removed from base and (right now) live here

@coveralls
Copy link

Coverage Status

Coverage decreased (-20.61%) to 76.51% when pulling 88a4012 on spencerlyon2:patch-1 into 2c7a96c on timholy:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-15.86%) to 81.26% when pulling 88a4012 on spencerlyon2:patch-1 into 2c7a96c on timholy:master.

@timholy
Copy link
Owner

timholy commented Mar 14, 2015

Thanks!

I'm a little puzzled it passes tests on both 0.3 and 0.4. Would it make more sense, though, to load the package only on 0.4? It looks like 0.4.0-dev+3066 is the version number in which the removal happened.

@tomasaschan tomasaschan mentioned this pull request Apr 8, 2015
@tomasaschan
Copy link
Collaborator

@timholy Do you want to load the package only on 0.4 before this merges? I don't think there's a problem with loading it on 0.3 as well - there's no require file or anything in WoodburyMatrices that specifies that it's 0.4 only (even though I know the same functionality exists in base).

I'm curious to see if merging this fixes the 0.4 builds on the other PR's, so if you don't want to merge this without that change and @spencerlyon2 doesn't have time for this right now, I might build another PR from this one which only loads the Woodbury package on builds after the specified commit.

@sglyon
Copy link
Contributor Author

sglyon commented Apr 8, 2015

Can we do a check of the Julia version from within REQUIRE?

@tomasaschan
Copy link
Collaborator

@spencerlyon2 I don't think that is possible, but I don't see a reason to given that it seems to work fine on 0.3 as well (and the need to is only temporary until 0.4 is released).

@timholy Do you have any objections to me merging this in its current state? I want this in before I merge #57.

@@ -1,6 +1,7 @@
module Grid

using Compat
using WoodburyMatrices
Copy link
Owner

Choose a reason for hiding this comment

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

You should wrap this in

if VERSION < v"0.4.0-dev+3066"
    using WoodburyMatrices
end

@tomasaschan
Copy link
Collaborator

@spencerlyon2 I took the liberty of cherry-picking your commits onto a branch of my own, which I merged in #58 - didn't feel like waiting :) Hope you're OK with that.

@sglyon
Copy link
Contributor Author

sglyon commented Apr 13, 2015

Of course, thanks for taking care of it. 

On April 13, 2015 at 6:56:12 AM EDT, Tomas Lycken notifications@github.com wrote:@spencerlyon2 I took the liberty of cherry-picking your commits onto a branch of my own, which I merged in #58 - didn't feel like waiting :) Hope you're OK with that. —Reply to this email directly or view it on GitHub.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants