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

Change all Array and Vector to AbstractArray and AbstractVector #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

biogeo
Copy link

@biogeo biogeo commented Feb 24, 2016

Changing all Array and Vector type specifications in method signatures to AbstractArray and AbstractVector to make code more generic, as per #86.

@mlubin
Copy link
Collaborator

mlubin commented Feb 24, 2016

Could you add some tests of this functionality?

@biogeo
Copy link
Author

biogeo commented Feb 24, 2016

Sure, I'm not sure exactly what to test though. Is it reasonable to add a dependency on, e.g., DataFrames for the purposes of testing with a DataArray?

@mlubin
Copy link
Collaborator

mlubin commented Feb 24, 2016

You could add a testing dependency on DataFrames (i.e., in test/REQUIRE). You could also just test with subarrays.

@biogeo
Copy link
Author

biogeo commented Feb 24, 2016

Sounds good, will do when I have a chance.

@biogeo
Copy link
Author

biogeo commented Feb 24, 2016

I added tests, essentially by replicating existing tests using SubArrays instead of Arrays.

@biogeo
Copy link
Author

biogeo commented Feb 24, 2016

Travis tests for Julia nightly release seem to be failing, but not on a line that makes sense to me based on anything I changed.

@biogeo
Copy link
Author

biogeo commented Feb 25, 2016

I can confirm that I get the same test failure when running Pkg.test on Julia 0.5.0-dev+2440 (the current dev version on JuliaBox) on the current master branch, so I don't think the failed test is anything to do with this PR.

@pkofod
Copy link

pkofod commented Dec 20, 2016

I guess this requires changing sub to view? Or include some mechanism for supporting both v0.4 and v0.5 ?

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

Successfully merging this pull request may close these issues.

3 participants