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.

Sign up for GitHub

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

Jump to bottom

Remove S4 classes #269

Open
joshuaulrich opened this issue May 16, 2019 · 4 comments
Open

Remove S4 classes #269

joshuaulrich opened this issue May 16, 2019 · 4 comments

Comments

@joshuaulrich
Copy link
Owner

The setOldClass("xts") and setOldClass("zoo") calls in quantmod cause issues if other packages need to define those classes as well. For example, see edzer/spacetime#35, and a StackOverflow question I answered a couple years ago.

I mentioned these to @jaryan and he would prefer to remove all the S4 classes from quantmod.
@bastistician
Copy link

I guess the following comment from the sources of the methods package also holds for "xts":

https://github.com/wch/r-source/blob/be05889aa5646e2c456486fd584be873ef3acd62/src/library/methods/R/BasicClasses.R#L531-L532

So maybe xts itself could setOldClass("xts") ?

@joshuaulrich
Copy link
Owner Author

Thanks for the suggestion @bastistician. That thought crossed my mind as well. Would you have time to try that and report back?

@nguinasso
Copy link

Has anyone fixed this yet? It is still bothering me.

joshuaulrich added a commit to joshuaulrich/xts that referenced this issue Oct 16, 2021
@joshuaulrich
Add and export xts S4 class
8ca448e 
A few packages need xts as a S4 class. I know quantmod and spacetime
both create an xts S4 class. The class definition belongs in the
package that owns the class.

See joshuaulrich/quantmod#269
joshuaulrich added a commit that referenced this issue Oct 16, 2021
@joshuaulrich
Move xts S4 class to xts package
4f5e6b7 
This removes the xts S4 class definition, and imports it from xts.
This requires an xts upgrade (xts > 0.12.1).

See #269.
@joshuaulrich
Copy link
Owner Author

@nguinasso I just tried to address it. It's going to take coordination with other package authors to get it released, so that may take several weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@joshuaulrich @bastistician @nguinasso