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

Decide on which functions to make internal #79

Open
athowes opened this issue Jun 6, 2024 · 4 comments · Fixed by #300
Open

Decide on which functions to make internal #79

athowes opened this issue Jun 6, 2024 · 4 comments · Fixed by #300
Assignees

Comments

@athowes
Copy link
Collaborator

athowes commented Jun 6, 2024

Currently (approximately) all of the functions in the package are external (@export).

We should decide which should be made internal to the package.

As example, in #70 @seabbs raised this regarding functions in utils.R and the epidist_stancode functions.

@athowes
Copy link
Collaborator Author

athowes commented Oct 7, 2024

PR #365 reopened this issue:

  • @seabbs suggests that we might want to consider making more functions internal, and that only those functions for a call to epidist() that a user might need to modify to add a new model, family, ... funcitonality should be exposed
  • Sam and I both feel unsure / not strongly about it
  • I think this change should apply uniformly to all of our functions. Especially going to be relevant for epidist_family, epidist_prior, epidist_formula (eventually when it's S3ed more), epidist_stancode

@seabbs
Copy link
Contributor

seabbs commented Oct 7, 2024

Noting that new functions introduced in #365 have been exported under the understanding that are reviewed here

@athowes
Copy link
Collaborator Author

athowes commented Nov 7, 2024

To create a new model aaa which has custom families and formula you need to:

  • as_aaa with as_aaa.epidist_linelist etc.
  • assert_aaa_input
  • epidist_validate.epidist_aaa
  • epidist_family_model.epidist_aaa
  • epidist_formula_model.epidist_aaa
  • epidist_stancode.epidist_aaa

To add a new reparameterisation of a family bbb you need to edit:

  • epidist_family_reparam.bbb

So in this sense, you perhaps don't need to edit epidist_formula or epidist_family or epidist_stancode to do these tasks.

In the case of epidist_stancode it's a generic and you would need to edit the method. So I suppose this would mean not exporting any of the methods either.

Similarly, if we are not to be exporting e.g. epidist_formula, does it make sense to be exporting epidist_formula_model?

Perhaps this line of thinking is pushing us towards not exporting any of these things?

@seabbs
Copy link
Contributor

seabbs commented Nov 13, 2024

@seabbs make sure to think about the simulation code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants