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

Tidy Output #6

Open
MartinHinz opened this issue Jul 14, 2017 · 16 comments
Open

Tidy Output #6

MartinHinz opened this issue Jul 14, 2017 · 16 comments
Assignees

Comments

@MartinHinz
Copy link
Member

implement functions that extract informations from the resulting oxcAAR objects and present them in easily processible formats (vector, data frame)

Candidates are:

  • raw probabilities (dataframe of dates and probabilities)
  • sigma ranges
    ...?
@FFaupel
Copy link
Member

FFaupel commented Jul 14, 2017

As I was busy during your report on oxcAAR with mortAAR, I am not up to date... Anyway, perhaps I have an idea for output once I read the vignette.

Sorry, for not listening ; )

@MartinHinz
Copy link
Member Author

OK, going the other way round, what is in the object and what can potentially be extracted:

  • name
  • bp
  • std
  • cal_curve
  • sigma_ranges
  • raw_probabilities

together with get, this might form a body of output functions. Not sure if name is relevant. So

  • get_bp(oxcAARCalibratedDate)
  • get_std(oxcAARCalibratedDate)
  • get_cal_curve(oxcAARCalibratedDate)
  • get_sigma_ranges(oxcAARCalibratedDate)
  • get_raw_probabilities(oxcAARCalibratedDate)

What do you think? Enough, to much, or any idea about a derivative value that would benefit from an explicit function?

@nevrome
Copy link
Member

nevrome commented Jul 21, 2017

Ok - I still don't understand oxcAAR completely, so correct me if I'm wrong.

There are two main classes: oxcAARCalibratedDates and oxcAARCalibratedDatesList

The tidy output should be a data.frame. I suggest to prepare an output data.frame were one row represents one date. In case of the oxcAARCalibratedDates, the output data.frame respectively has one row, in case of the oxcAARCalibratedDatesList many.

These are the values in oxcAARCalibratedDates:

#' @param name a string giving the name of the date (usually the lab number)
#' @param bp a integer giving the BP value for the date
#' @param std a integer giving the standard deviation for the date
#' @param cal_curve a string giving the used calibration curve
#' @param sigma_ranges a list of three elements (one, two, three sigma), each a data frame with start, end and probability giving
#' @param raw_probabilities a data frame of dates and the related probabilities for each date

Strings and integers can go into the data.frame immediately, the list and the data.frame can be stored in list columns. That's how I would do it. D'accord?

@nevrome
Copy link
Member

nevrome commented Jul 21, 2017

Idea: We don't write an own tidy output function but prepare a pull request with one for the broom package. https://github.com/tidyverse/broom

@MartinHinz
Copy link
Member Author

MartinHinz commented Jul 21, 2017

Now I understand much better what you have in mind.

What about the following:

  • For oxcAAR 1.0 we implement getter for the individual values
  • For oxcAAR 1.0 optional, for oxcAAR 1.1 mandatory, we implement a get_tidy_oxcalresult function that returns a 'tidyverse' version of our output
  • Parallel to that we implement a interface at broom that uses the build_in functions

Benefits:

  • Independent from broom, but still connected
  • more encapsuled and isolated
  • tidy and traditional available

Costs:

  • more work

@nevrome
Copy link
Member

nevrome commented Jul 21, 2017

get_tidy_oxcalresult() should be part of 1.0. We tackle the broom::tidy() implementation when oxcAAR 1.0 is on CRAN. Let's do it!

@MartinHinz
Copy link
Member Author

MartinHinz commented Jul 21, 2017

50 % reached with e4af874. Yeahh!

Now the get_tidy_oxcalresult() function has to be implemented.

@MartinHinz
Copy link
Member Author

Returned to thinking about the usefulness of this... It would make it necessary to import "tibble", just for this purpose. Especially questionable, when we still want or need to implement a function at broom. So I changed my mind, and would vote for transfering all tidy functionalitity to our broom fork. What do you think?

@nevrome
Copy link
Member

nevrome commented Jul 23, 2017

I'm pretty confident that we don't need tibble for the basic implementation. Further on get_tidy_oxcalresult() could already be structured more or less like the future tidy() implementation. I'll give it a go this afternoon and you can decide afterwards if you like it, ja?

@MartinHinz
Copy link
Member Author

Sure, would have done that my self, but fine. But I still do not know how you will do that without the actual tibble format!? How put the data.frame of probabilities into a "data.frame" of a tidy structure!?

@nevrome
Copy link
Member

nevrome commented Jul 23, 2017

A normal dataframe also allows nested content. Do I miss something?

data.frame(
  a = 1:3, 
  b = I(list(1,1:2,1:3)), 
  c = I(list(list(1,1:2,1:3), list(1,1:2,1:3), list(1,1:2,1:3))), 
  d = I(list(mtcars, mtcars, mtcars))
)

I'm still working on the getter functions, but eventually I'll get to it...

@MartinHinz
Copy link
Member Author

Right, damn, my ignorance. Was not aware of the I! OK, then it seems that I am looking forward to your solution!!

@nevrome
Copy link
Member

nevrome commented Jul 23, 2017

af2cc5c. You're right - would be much more nice with tibble. But I think that'll do for the moment until we have a CRAN release and can take a look at broom.

Was not in the mood to write tests yet. Will do later.

Concerning the getter functions: Does the call to unname() make sense?

@MartinHinz
Copy link
Member Author

https://www.youtube.com/watch?v=BHsSgxpX8rY

Thanks alot, could also do the tests if you like, let me know.

Unname absolutely makes sense!

@nevrome
Copy link
Member

nevrome commented Jul 23, 2017

Haha - nono I can write the tests. ;-)

@MartinHinz
Copy link
Member Author

Do you think it is ok to move this issue than to 1.1?

@MartinHinz MartinHinz modified the milestones: oxcAAR 1.1, oxcAAR 1.0 Jul 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants