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

shiny feature needs to be implemented as a function #3

Open
thibautjombart opened this issue Feb 16, 2015 · 11 comments
Open

shiny feature needs to be implemented as a function #3

thibautjombart opened this issue Feb 16, 2015 · 11 comments
Assignees

Comments

@thibautjombart
Copy link
Member

Current shiny stuff has been moved to /inst/shiny. Some changes are needed for users to be able to access the shiny features.

We would need:

  • to check that the files in /inst/shiny are not redundant
  • to make this a function where the dataset can change; code currently uses a fixed dataset
  • calls to 'library' have to be removed from within the .R files and stated as dependencies (fields @import in roxygen and adding the relevant packages to DESCRIPTION file)

One way to do this is:

  • have the files server.R and ui.R in the inst/shiny folder
  • have a function starting the server which refers to these files using system.files()

There is an example of that with the function adegenetServer() in adegenet:

@rolinavg
Copy link

Hi Thibaut,

I've divided the script file into ui.R and server.R files, but now I'm not sure what to do with the system.files() command and how to pass variables to these files without calling them as source code. Can you work your magic? How am I supposed to commit my changes now? I tried to git pull origin shinyFeatures, but it said the following:

git pull origin shinyFeatures
From https://github.com/Hackout2/incidence

  • branch shinyFeatures -> FETCH_HEAD
    Updating d851b54..8d83195
    error: Your local changes to the following files would be overwritten by merge:
    R/clickableTimeSeries.R
    Please, commit your changes or stash them before you can merge.
    Aborting
    <<<<<<<<<<<

I think it would be cool to add maps to the interactive time series. Any other thoughts on improving this code are naturally welcome!

Rolina

@thibautjombart
Copy link
Member Author

Hi Rolina,
Cool! Did you check the links to adegenet's stuff I posted? This one shows how to refer to the installed files when calling runApp:
https://sourceforge.net/p/adegenet/code/ci/master/tree/pkg/R/servers.R

@thibautjombart
Copy link
Member Author

As for the git error, this is because you probably haven't committed your changes in your local branch. You have to commit first, then pull, then push. Otherwise pulling would erase the changes you haven't committed. Makes sense?

@rolinavg
Copy link

Hi Thibaut,

That last comment makes sense, yes. As for the example files you posted, I did indeed look at those, but it wasn't clear to me how user-defined variables are passed to ui.R and server.R. Would you put them in the brackets at line 6 or at line 20 (line references refer to your example code)? And how about code that is neither in the ui.R file nor server.R file and is necessary to modify the user-defined variables? Where does this go? Finally, presumably the server.R location should be inst\shiny and I should reference the incidence package at line 21. Correct?

Thanks!
Rolina

@thibautjombart
Copy link
Member Author

Hi Rolina,
I'm not sure about the passing of variables, I guess it depends which ones.
Passing the dataset directly when calling shiny should be possible, but I
haven't done this yet. As for referring incidence, it should not be the
case: the code will be run, by definition, when the package is attached.
If you want to push what you have so far, I'll have a look later.
Cheers
Thibaut

On Thu, Feb 19, 2015 at 1:52 PM, rolinavg notifications@github.com wrote:

Hi Thibaut,

That last comment makes sense, yes. As for the example files you posted, I
did indeed look at those, but it wasn't clear to me how user-defined
variables are passed to ui.R and server.R. Would you put them in the
brackets at line 6 or at line 20 (line references refer to your example
code)? And how about code that is neither in the ui.R file nor server.R
file and is necessary to modify the user-defined variables? Where does this
go? Finally, presumably the server.R location should be inst\shiny and I
should reference the incidence package at line 21. Correct?

Thanks!
Rolina


Reply to this email directly or view it on GitHub
#3 (comment).

@rolinavg
Copy link

I see. Ok. Perhaps I'll revert to the one-file function-friendly format that I had developed and just push that for now. In this one file, shiny is called within a normal R function, allowing me to pass whichever user-defined variables I want. I'll push it later this evening when I get home or tomorrow.

Rolina

@rolinavg
Copy link

Hi Thibaut,
I've committed / pulled / pushed my code onto the shinyFeatures branch. I hope I did it correctly. Had some error messages pop up, but google seemed to provide a useful solution. I still need to edit and modify a few things. Feel free to have a look when you have the chance.
Rolina

@thibautjombart
Copy link
Member Author

Hi Rolina,
cool, I can see the commit now, thanks!
Cheers
Thibaut

On Thu, Feb 19, 2015 at 10:23 PM, rolinavg notifications@github.com wrote:

Hi Thibaut,
I've committed / pulled / pushed my code onto the shinyFeatures branch. I
hope I did it correctly. Had some error messages pop up, but google seemed
to provide a useful solution. I still need to edit and modify a few things.
Feel free to have a look when you have the chance.
Rolina


Reply to this email directly or view it on GitHub
#3 (comment).

@thibautjombart
Copy link
Member Author

Sorry I have left this hanging for a while. @rolinavg I probably won't have time to work on the shiny app over the days to come. What do you think? Shall we make the first release without this functionality? Or would you rather sort it out first?

@rolinavg
Copy link

Hi Thibaut,

Sorry for the late reply! I thought that I might have time to work on it on
Friday or during the weekend, but it turns out that those days were more
fully booked than I anticipated. I may have some time to look at it this
weekend, but am rather swamped until then. Will this be okay?

Rolina

On Thu, Mar 26, 2015 at 1:18 PM, Thibaut Jombart notifications@github.com
wrote:

Sorry I have left this hanging for a while. @rolinavg
https://github.com/rolinavg I probably won't have time to work on the
shiny app over the days to come. What do you think? Shall we make the first
release without this functionality? Or would you rather sort it out first?


Reply to this email directly or view it on GitHub
#3 (comment).

@thibautjombart
Copy link
Member Author

Hi Rolina,
sure!
Best
Thibaut

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

2 participants