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

WIP: adds conversion from TimeArray -> AxisArray, and various other methods #13

Closed
wants to merge 3 commits into from

Conversation

milktrader
Copy link

Abandoning the brainstorm to use nested modules and taking @tshort approach to using @require.

This branch will provide the ability to experiment with time-flavored AxisArrays and begins work on some related methods

  • merge
  • moving (might name this differently)

Also, the tests are not part of the overall test suite but ensure that conversions are happening correctly.

m = length(a)-n+1
res = zeros(m)
for i in 1:m
res[i] = f(a[Interval(a.axes[1][i],a.axes[1][i+n-1])])
Copy link
Member

Choose a reason for hiding this comment

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

This can be expressed without the Interval — simply f(a[Axis{1}(i:i+n-1)])

Copy link
Author

Choose a reason for hiding this comment

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

Much nicer.

Copy link
Author

Choose a reason for hiding this comment

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

The refactor passes ...

julia> @test AxisArrays timeseries
conversion from TimeArray to AxisArray
     - axes values are correct
     - data values are correct
5 facts verified.
moving
     - moving only works on single column
2 facts verified.
merge
     - only inner join supported
1 fact verified.

@mbauman
Copy link
Member

mbauman commented Mar 4, 2015

Awesome. I've been thinking a bit about this. Here's how the moving function might look if it were generalized to work over any dimension:

function moving{id}(a::AxisArray, f::Function, n::Axis{id,Int})
    Ax = Axis{id}
    v = n.val
    res = similar(a, Ax(axes(a, Ax).val[cld(v,2):end-fld(v,2)]))
    for i in 1:size(res, Ax)
        res[Ax(i)] = f(a[Ax([i:i+v-1]))
    end
end

You'd call it like this: moving(AAPL, mean, Axis{:time}(10)) for a 10-day moving average across the time axis.

Of course, this requires my custom indexing patch (#11), as well as a specialized similar function that I've not written yet to return a new AxisArray with modified dimensions.

Edit: updated to reflect the new work in 7876a32.

One issue in the general case is that we need to tell f which dimension to work over. I suppose we could require that f be like mean/median/etc and take the dimension as a second argument.

@mbauman
Copy link
Member

mbauman commented Mar 5, 2015

It's very interesting to me how close that general method is to allowing image processing… you could easily extend it to allow an arbitrary number of axes (with some staging) to define a moving square or cube through your data. If we used mean!/median! then the indexing shapes themselves would define the reduction dimension(s).

With that said, I think the most performant way to do a running mean would be through filtering or fft techniques (as a convolution of a weighted vector). And there are specialized methods for running median, too. But this generic case is really useful in thinking through the AxisArray APIs.

I really don't like Ax(axes(a, Ax).val[cld(v,2):end-fld(v,2)]). Perhaps axes could allow for direct indexing?

Ax(axes(a, Ax(cld(v,2):size(A,Ax)-fld(v,2)))) # Not really much better

Or if we make Axis <: AbstractVector:

Ax(axes(a, Ax)[cld(v,2):end-fld(v,2)])

@mbauman
Copy link
Member

mbauman commented Sep 15, 2016

I'm going to close this for now; it's out of date and I think we need to move away from @require method of lazy dependencies. Feel free to resurrect if you ever are interested in using AxisArrays. Thanks for helping me brainstorm how this package works!

@mbauman mbauman closed this Sep 15, 2016
@milktrader
Copy link
Author

Good call.

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.

2 participants