Why timestamp instead of timestamps?
It seems that timestamp(ts) and value(ts) or timestamps(ts) and values(ts) would be more consistent. (a minor issue)
eh
Yeah, not sure how much of others' code it would break. Of course we could go the deprecation route to mitigate that issue.
@milktrader thanks for the note .. let's deprecate it -- the advantage is to new users, and naturalness in soft-terms is its own reward.
also ..
We have a deprecated.jl file in the /src directory.
Both plural or both singular?
It's more of a stretch to say values are a value than it it to say a timestamp is timestamps so I would vote for going plural.
timestamp -> timestamps
We already have values
Also fwiw, the original names come from the name of the type parameters.
ts.values
ts.timestamp
both plural because it is a rare event to have a TimeArray with a single timestamp and a values array that is a vector with one entry
The underlying reason to use the plural forms here parallels our convention to name a module that exports a type in the plural so the exported type can be named in the singular (IndexedTables, IndexedTable). TimeArrays hold two fields that provide any of many valuations specific to the fields. they are plural.
After thinking about this for a few days I'm not sure we need to change the timestamp method name to timestamps. Here is my reasoning.
timestamp is a field of the TimeArray type. The timestamp method is nothing more than a wrapper for that field.
###### element wrapers ###########
timestamp{T,N}(ta::TimeArray{T,N}) = ta.timestamp
values{T,N}(ta::TimeArray{T,N}) = ta.values
colnames{T,N}(ta::TimeArray{T,N}) = ta.colnames
meta{T,N}(ta::TimeArray{T,N}) = ta.meta
The change of timestamp to timestamps would stand out as incongruous and odd. I do get the point that it's a stretch to think of timestamp as a series of timestamps. If anything, I would be in favor of removing this code altogether.
There has been an effort to encourage exactly that use of fieldnames as functional accessors to the verysame field -- and to discourage immediate use of the .fieldname form outside of very low level ops. AFAIK that is still the preference -- but I have not discussed it recently. Some of the desire is to protect the specifics of a type implementation from becoming necessary furnature.
On Mon, Jun 12, 2017 at 7:20 AM, milktrader [email protected] wrote:
After thinking about this for a few days I'm not sure we need to change the timestamp method name to timestamps. Here is my reasoning.
timestamp is a field of the TimeArray type. The timestamp method is nothing more than a wrapper for that field.
element wrapers
timestamp{T,N}(ta::TimeArray{T,N}) = ta.timestampvalues{T,N}(ta::TimeArray{T,N}) = ta.valuescolnames{T,N}(ta::TimeArray{T,N}) = ta.colnamesmeta{T,N}(ta::TimeArray{T,N}) = ta.meta
If anything, I would be in favor of removing this code altogether.
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/JuliaStats/TimeSeries.jl/issues/315#issuecomment-307761585, or mute the thread https://github.com/notifications/unsubscribe-auth/ABmqxgmygz7gPQNQa_FTvVom9DumZq6Sks5sDR7igaJpZM4NzTHM .