mfx tidiers

[grantmcdermott]

Addresses the first part of #700

Some things worth noting / still need to be addressed:

• I squashed everything into a generic set of tidiers for any mfx object, since they all accept the same arguments. Let me know if you would prefer these broken out into their distinct classes. (i.e. Separate files for classes betamfx, logitmfx, etc.
• Some tests are currently failing because of missing glossary terms as per Add tidiers for margins and mfx objects #700 (comment). Let me know if you want to add the glossary terms yourself. Otherwise I'm happy to do it. UPDATE: Done, pending this PR Add glossary term for mfx_tidiers alexpghayes/modeltests#22
• I'm getting a warning about: Undocumented arguments in documentation object 'mfx_tidiers': ‘data’ ‘newdata’ ‘type.predict’ ‘type.residuals’ ‘se_fit’ when I run devtools::check(). I'm really not sure why this is, but hopefully you can spot the problem quickly. UPDATE: Fixed.
• Similarly, a warning about Rd files with duplicated alias 'mfx_tidiers': ‘augment.mfx.Rd’ ‘glance.mfx.Rd’ ‘mfx_tidiers.Rd’. My limited roxygen2 skills have stumped me here, so again I hope that's an easy fix for someone with more experience. UPDATE: Fixed.
• I couldn't get the check_augment_function() test for augment.betamfx to work even though everything looks good to me. (See here.) There's some deeper work that probably needs to be done with the underlying modeltests package to ensure compatibility here, so I just commented it out for the moment. UPDATE: Fixed.
• Last, very minor comment: I couldn't see any logic to the ordering of contributors, alphabetic or otherwise, in the DESCRIPTION file. So I just added my name at the top. Feel free to rearrange as package etiquette demands ;)

[alexpghayes]

Can you fix the NEWS conflicts and hopefully trigger a new Travis build in the process?

On the whole, this looks great. Thank you for your contribution!

[alexpghayes]

Looks like there's a failing test https://travis-ci.org/tidymodels/broom/builds/624816058#L1894

[grantmcdermott]

Looks like there's a failing test https://travis-ci.org/tidymodels/broom/builds/624816058#L1894

Ah, it appears that the explicit use of glm.glance() — suggestion here — is now causing the glance.betamfx() case to break. (Same for augment.betamfx().) I guess we have two options:

1. Revert back to a generic glance() shim and let broom figure things out based on the underlying object fit. This is what I had previously.

2. Check whether the object is of the betareg class (probably just use a regexp on the model call) and explicitly use separate glance.betareg() and augment.betareg() tidiers for these cases.

Do you have a preference?

UPDATE: Upon reflection it's going to have to be option 2. In fact, I'll need to break the augment.betamfx and glance.betamfx methods into their own categories because they accept different arguments. (The remaining objects all take exactly the same glm methods and arguments.) I'll work on this when I get a chance this afternoon. Thanks for bearing with me.

[grantmcdermott]

@alexpghayes Okay, I've separated out the betamfx tidiers from the rest. Everything looks good when I run devtools::check() on my local machine, so hopefully it passes the Travis CI check now too.

[grantmcdermott]

Hey @alexpghayes (and @simonpcouch) revisiting this PR after a looong absence. Near as I can tell, everything looked good on my side after the last review changes. I know that you've obviously pushed various updates to the master branch since. Let me know if you need me to merge upstream and then, hopefully, trigger a new CI build.

[alexpghayes]

Yeah can you merge upstream in one more time? Sorry for the criminal delay on this.

[alexpghayes]

I'm happy to merge as soon as the conflict is resolved!

[grantmcdermott]

Hmmm. Looks like some tests are failing on the Windows build for weird (unrelated?) reasons; e.g. ggplot2 failed to install as a dependency. Is this something other PRs have encountered?

[simonpcouch]

@grantmcdermott Thanks for making that happen!

Yeah, really strange--added some contributors to DESCRIPTION and that ggplot2 issue came up. Not your fault.🙂

Will merge this afternoon!

[grantmcdermott]

Hi @simonpcouch

I just filed an issue (#873), but I just tried running this on the dev version and ran into a weird error about the mfx tidiers not being found. I'm fairly stumped since this worked fine on my local branch and also seemed to pass all the relevant CI tests. I know you've got a lot on your plate but wanted to make you aware since the PR has already been merged. Any ideas?

[github-actions]

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.