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

Issue #56 #57

Merged
merged 7 commits into from
Jan 30, 2025
Merged

Issue #56 #57

merged 7 commits into from
Jan 30, 2025

Conversation

JanOkul
Copy link
Contributor

@JanOkul JanOkul commented Jan 21, 2025

This mostly resolves #56, porting the Retistruct GUI in R Shiny, however there is still some work to be completed.

Tasks that have so far been completed:

  • Added in server.R, ui.R, app.R, and server-handlers.R which are the Shiny components.
  • Changed retistruct.R and ReconstructedOutline.R. The changes mainly add the shiny output object as arguments so that the plots can be rendered and assigned to the output object, instead of a (now non-existent) graphics device. As well as this, the if guards for the plotting have been changed from checking if a graphics device exists, to if an output object exists.
  • Removed the old GUI file.

Tasks still left to do/issues to fix:

  • Implement the save as Bitmap/PDF feature for both plots.
  • Implement the loading packages on retistruct() call.
  • Implement an error handler that works with Shiny better.
  • Change package files to remove redundant dependencies.
  • Removing too many tears results in the left plot crashing.
  • When adding a tear, the marked points no longer highlight due to the built-in identify() function no longer being used.
  • Add a max distance parameter to the custom identify function used.

…upport, modified ReconstructedOutline.R and retistruct.R to pass in the shiny output object, to allow rendering support for shiny, removed the old gui file.
…d removed old dependencies from DESCRIPTION, removed root volume from shinyFiles directory select.
Copy link
Owner

@davidcsterratt davidcsterratt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments in code.


report(paste("Mapping optimised. Deformation energy E:", format(self$opt$value, 5),
";", self$nflip, "flipped triangles."))
},
},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Picky, but remove white space at end of line.

datapoints=FALSE, landmarks=FALSE,
image=FALSE)
})
} else if (!is.na(dev.flat)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this needs to be "else if". I suppose we might want to do both, though it's unliikely.

datapoints=FALSE, landmarks=FALSE,
image=FALSE)
})
} else if (!is.na(dev.flat)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment about "else if".

@@ -0,0 +1,11 @@
##' Start the Retistruct GUI
##' @seealso gWidgets2
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this line - we're not using gWidgets any more.

##' @importFrom shiny shinyApp
##' @export
retistruct <- function() {
options(rgl.useNULL = TRUE) ## Prevents rgl from making it's own window
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very picky: it's -> its

@@ -0,0 +1,388 @@
## Converts projections into a function, as values returned from ui are strings.
translateProjections <- function() {
return(list('0' = azimuthal.equidistant,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Picky: I would probably use double quotes here to match code elsewhere.

##' @description The R shiny server responsible for storing a state for each
##' session, handling inputs from the UI to the server, and plotting outputs
##' to the UI. The arguments are all handled by the shiny package and this
##' function should not be insantiated manually.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insantiated -> instantiated

(picked up by the spell checker)

@davidcsterratt davidcsterratt merged commit 9d852a2 into davidcsterratt:master Jan 30, 2025
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

Successfully merging this pull request may close these issues.

Port GUI to R shiny
2 participants