Fails to construct 'xts' object with additional classes

[shikokuchuo]

An 'xts' object is just a matrix with a time index. This example works fine with dput but fails with constructive (and it is not apparent from the error message why).

constructive::construct(ichimoku::ichimoku(ichimoku::sample_ohlc_data))
#> Error in `constructive::construct()`:
#> ! {constructive} could not build the requested code.
#> Caused by error in `if (formatted == "NA") ...`:
#> ! the condition has length > 1

^{Created on 2024-07-09 with reprex v2.1.0}

If this can't be handled, perhaps the error message can be improved, or else fall back to dput with a warning?

[moodymudskipper]

Thanks for breaking constructive :D. We can definitely handle this.

[moodymudskipper]

This should be fixed by #454

Meaning it won't fail and reproduce the object accurately (as a matrix with a class). To use idiomatic constructors there's a bit more work:

There are many constructors that can be used to build a xts object :

• as.xts.timeSeries()
• as.xts.ts()
• as.xts.zoo()
• as.xts.data.frame()
• as.xts.matrix()
• xts()
• .xts()

I'm not sure what exactly should be the default one but the example uses as.xts.matrix() so I think we can start with this one.

We also want to support objects of class timeSeries and zoo in another ticket.

Would be nice to release a patch of minor version with all the ts stuff handled at once if it's not too hard.

[shikokuchuo]

Thanks, can confirm that PR fixes the issue.

I like how it is constructed, as it highlights the simplicity of the internal structure of an xts/zoo object, which is just a matrix with am index attribute. But of course I am biased as this is exactly how I construct these objects at the C level.

A matrix converted using as.zoo / as.xts makes sense as an idiomatic constructor.

[moodymudskipper]

Yes, I've implemented the constructors already, as.xts() on a data.frame looks the best but I would need to depend on the xts package to make it the default constructor, so it'll be as.xts() on matrices instead. I need to add some tests and push. I'll implement zoo constructors afterwards.

On Wed, 10 Jul 2024, 16:34 Charlie Gao, @.***> wrote:

Thanks, can confirm that PR fixes the issue.

I like how it is constructed, as it highlights the simplicity of the internal structure of an xts/zoo object, which is just a matrix with am index attribute. But of course I am biased as this is exactly how I construct these objects at the C level.

A matrix converted using as.zoo / as.xts makes sense as an idiomatic constructor.

— Reply to this email directly, view it on GitHub https://github.com/cynkra/constructive/issues/453#issuecomment-2220685851, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEMAMYSPQHD7IERL4IF5ELDZLVBBHAVCNFSM6AAAAABKTXBE6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRQGY4DKOBVGE . You are receiving this because you commented.Message ID: @.***>