dysonance / Temporal.jl

Time series implementation for the Julia language focused on efficiency and flexibility
Other
100 stars 26 forks source link

Modify column name of "Adj Close" #20

Closed Balinus closed 4 years ago

Balinus commented 6 years ago

Hello,

right now it does not seems possible to plot the column"Adj Close" because of the whitespace. Adj Close should be renamed to perhaps "Adj_Close" so that we can plot it with

plot(data[:Adj_Close])

dysonance commented 6 years ago

Hi @Balinus,

Thanks for the heads up, sorry you're having trouble. Can you maybe better help me understand the issue you're having?

When I try to plot TS objects with whitespace in the fields, the legend does show all columns including "Adj Close" appropriately:

using Temporal
data = yahoo("SNAP")
using Plots
plot(X[:,1:5])

Is it that you are having trouble selecting/indexing the "Adj Close" field specifically? You should be able to get that column with data[Symbol("Adj Close")]. Does this solve your problem?

On a potentially related note, there was indeed a recent change to the package to keep from "sanitizing" the names when the TS objects are created. That change was motivated by this issue).

dysonance commented 6 years ago

@Balinus Also worth noting there is a convenience function defined that might be able to help you out as well for getting the closing prices column of a TS object: adj_close = cl(data)

Balinus commented 6 years ago

Thanks for the answers. My aim is to indeed select/index the Adj Close field for plotting but also for analysis.

Going with data[Symbol("Adj Close")] is too verbose imho. I'd rather type in the REPL something quicker. Although I could write some wrapper function similar to cl(data) (which return the Close colum and not the Adj Close btw).

In the end, it's not very important, but rather inconvenient that we can select other field with a direct command such as data[:Close] but not Adj Close which need his own command. This make the creation of analysis code more cumbersome than it should be.

Cheers!

dysonance commented 6 years ago

@Balinus Yes the issue with the cl(data) function returning the :Close column instead of adjusted is a bug that I just fixed last night, so that should be fixed for you soon. Sorry about that, but thanks for helping me catch the issue with your question. The cl function is indeed supposed to default to prefer the adjusted close prices instead of just the :Close column, and it should be all fixed now. So you can either wait for the change to be merged into METADATA or you can checkout Temporal's master branch and do a pull to get the most up-to-date change so you can use it ASAP.

The idea of a package constant or setting like SANITIZE_NAMES was discussed that would control the behavior of this functionality, and basically just have whitespaces (or non-alphanumeric characters generally) removed from the field names. What do you think?

I'm personally quite partial to that idea, as I totally agree it makes writing code way more cumbersome having to type out essentially twice as many characters just to say data[Symbol("Adj Close")] just to get a column. Something as simple as indexing a column should be way quicker in my opinion. Open to ideas to enhance the functionality from a user perspective.

Balinus commented 6 years ago

Thanks for your answer.

I think that just removing whitespaces would be enough and would ensure consistency in calling columns, without being too much restrictive. However, I see from the other thread your linked that other users want to have more flexibility. So perhaps it's just a matter of removing whitespaces when using the provided yahoo, google function to extract market data?

femtotrader commented 6 years ago

According docstring it was previously AdjClose https://github.com/dysonance/Temporal.jl/blob/9cba04755470b6c8d5d9c93f4217298e1cfd746d/src/io.jl#L288

A rename! function to rename column names is probably required

I don't know if such a function currently exists in Temporal

So instead of this code https://github.com/dysonance/Temporal.jl/blob/9cba04755470b6c8d5d9c93f4217298e1cfd746d/src/io.jl#L317

we could have something like

data = TS(indata[1], indata[2], indata[3][2:end])
rename!(data, Symbol("Adj Close")=>:AdjClose)
return data

See https://github.com/dysonance/Temporal.jl/issues/22 for such a rename! function implementation

femtotrader commented 6 years ago

A rename! function implementation could also accept a function as 2nd parameter which could rename column name in place.

femtotrader commented 5 years ago

Similar to https://github.com/JuliaQuant/MarketData.jl/issues/57 as TimeSeries.jl now uses Symbol for column indexing (like DataFrames.jl and Temporal.jl)

femtotrader commented 5 years ago

Function (in fact a Base.Callable) shouldn't be second parameter for renaming but first parameter to support do block see https://github.com/JuliaData/DataFrames.jl/issues/1578#issuecomment-434005580