search

ocbe-uio / TruncExpFam

R package to generate data related to the Truncated Exponential Family
https://ocbe-uio.github.io/TruncExpFam/
GNU General Public License v3.0
0 stars 1 forks source link

Constrain "trunc_" classes to output of `rtrunc()` #97

Closed wleoncio closed 1 year ago

wleoncio commented 2 years ago

Several generic functions on https://github.com/ocbe-uio/TruncExpFam/blob/36c134f038f770fa641d57786c8f9ee73330af8a/R/genericFunctions.R dispatch to trunc_-class object, and yet they have arguments like eta, which has a different structure than the output of rtrunc() so it arguably shouldn't be the same class.

One solution would be to require these functions to also have a y argument, whose purpose would just be to retrieve its class for dispatching.

Another solution would be to refactor the whole package to use RC or R6. This would make the rtrunc_ class a lot better structured, but breaks parallelism with the stats functions, because the syntax to call methods would be different.

Connected to the first idea, perhaps eta can be instead added as an attribute of y, so that only y is passed to the generic and eta is retrieved from it. The same would have to be done with the parms argument, but in this case the attribute must not conflict with the real-parameter attribute created during the rtrunc() call.

wleoncio commented 2 years ago

The goal is to remove any direct call of methods. The generics should always be called, even within the code.

wleoncio commented 1 year ago

To simplify the issue, these are the generics that are affected and need intervention:

All other generics are a function of y, which is correctly-structured as trunc_*-class objects.

wleoncio commented 1 year ago

Ok, so I tried the first idea of adding y to the generics, and it just breaks everything, so I don't think that's something worth spending much time on.

The idea of reimplementing classes also feels strange right now: R6 would introduce a dependency just for the sake of refactoring. RC would solve this, but then again, now I think using OOP would require unnecessary major restructuring, since each distribution would be a subclass of the truncated family, and perhaps the same things for the parameters.

Since the regular parameters are already written as attributes of the sample (y), what makes most sense to me right now is to just

rho62 commented 1 year ago

Me again

Can we make it tuesday at 11am instead?

/René

Fra: Waldir Leoncio @.***> Sendt: onsdag 8. november 2023 14:11:31 Til: ocbe-uio/TruncExpFam Kopi: Subscribed Emne: Re: [ocbe-uio/TruncExpFam] Constrain "trunc_" classes to output of rtrunc() (Issue #97)

Ok, so I tried the first idea of adding y to the generics, and it just breaks everything, so I don't think that's something worth spending much time on.

The idea of reimplementing classes also feels strange right now: R6 would introduce a dependency just for the sake of refactoring. RC would solve this, but then again, now I think using OOP would require unnecessary major restructuring, since each distribution would be a subclass of the truncated family, and perhaps the same things for the parameters.

Since the regular parameters are already written as attributes of the sample (y), what makes most sense to me right now is to just

— Reply to this email directly, view it on GitHubhttps://github.com/ocbe-uio/TruncExpFam/issues/97#issuecomment-1801864809, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFPRUPVOM7RLNLWZL4JLVODYDOAIHAVCNFSM6AAAAAASPSTQW6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBRHA3DIOBQHE. You are receiving this because you are subscribed to this thread.Message ID: @.***>

wleoncio commented 1 year ago

New idea, much cleaner than the suggestion above:

There's no reason for parameters2natural() and natural2parameters() to have methods for trunc_* classes, since they are not dependent on samples, only on parameters. Therefore, methods such as parameters2natural.trunc_normal() should be replaced with something like parameters2natural.parms_normal(). Thus, we create new parms_* superclasses just for the parameter arguments parms and eta, which are basic named vectors.