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

change arguments to output modules #383

Open
goldingn opened this issue May 3, 2017 · 4 comments
Open

change arguments to output modules #383

goldingn opened this issue May 3, 2017 · 4 comments

Comments

@goldingn
Copy link
Member

goldingn commented May 3, 2017

Output modules take two arguments, .model and .ras

  • .model is a list of two elements: df and model
    • df the dataframe returned by the last process module
    • model the ZoonModel object returned by the model module
  • .ras the raster* object re from process module(s)

This is confusing because:

  1. it seems unnecessary for df and model to be listed together
  2. .model is a confusing name for this list

I suggest we split .model$data up so that output modules take three default arguments:
.df, .ras, .model

This would require changing both zoon::workflow(), various references in the vignettes, zoon's tests, and the existing output modules.
The other functions, and the schematic in the zoon paper, should not be affected.

Can anyone think of a reason not to do this?
cc @AugustT

@AugustT
Copy link
Member

AugustT commented May 3, 2017

Seems fair to me. While you're there why not change the names? .df and .data are pretty much meaningless

@timcdlucas
Copy link
Contributor

The only reason I can think of is that it might be best for all module types to have one default argument, a list of all the or bits. This saves space in the man page etc.

But probably your plan is better, split things into sensible chunks.

@goldingn
Copy link
Member Author

goldingn commented May 3, 2017

@AugustT we discussed changing the names earlier, but couldn't think of anything better than df ras model. Open to suggestions though!

I kind of like the idea of making them all one standardised list. E.g. every module (or rather process, model & output modules) get the same first argument: .zoonInternal (or something), a list which always contains df, ras and/or model, depending on the module type.

so:

process modules would use .zoonInternals$df, .zoonInternals$ras
model modules would use .zoonInternals$df
output modules would use .zoonInternals$df, .zoonInternals$ras, .zoonInternals$model

thoughts?

@timcdlucas
Copy link
Contributor

I think it simplifies things. The list name will be used a lot, so the shorter the better. Maybe .data for the whole list.

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

No branches or pull requests

3 participants