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

Arima's namespace #2

Closed
helios opened this issue Jul 25, 2013 · 2 comments
Closed

Arima's namespace #2

helios opened this issue Jul 25, 2013 · 2 comments

Comments

@helios
Copy link

@helios helios commented Jul 25, 2013

module Statsample
module ARIMA
class ARIMA < Statsample::Vector

it seems to me a bit redundant I would use something more general for the ARIMA module to keep the class name ARIMA.
I saw that the same approach is present for Statsample::TimeSeries::TimeSeries

Name spaces should be browsable in the way a pleople easily find out what is he/she looking for or to understand the structure of the code base. Here it seems a bit misleading or, better, no so clear.
Looking at the wiki description I would use something like

module Statsample
class TimeSeries
module Model
class ARIMA < Statsample::Vector
end #ARIMA
class ARMA < ARIMA
end #ARMA
end # Model
end # TimeSeries
end #Statsample

@AnkurGel
Copy link
Owner

@AnkurGel AnkurGel commented Jul 26, 2013

This is very valuable concern, @helios!

Agreed, I will change the namespace hierarchy to something conventional.

@clbustos
Copy link
Contributor

@clbustos clbustos commented Aug 31, 2013

I suggest

module Statsample::TimeSeries # Module for all time series related stuff
class Statsample::TimeSeries::Series < Statsample::Vector # A vector storing a time serie
module Statsample::TimeSeries::Pacf # pacf related methods
class Statsample::TimeSeries::Arima # ARIMA class, should be initilized with a class method.

@AnkurGel AnkurGel closed this in a8e3c0d Aug 31, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.