Skip to content

Review #91

Merged
alex-sandercock merged 19 commits intoBreeding-Insight:cris_bridgefrom
Cristianetaniguti:main
Jan 30, 2025
Merged

Review #91
alex-sandercock merged 19 commits intoBreeding-Insight:cris_bridgefrom
Cristianetaniguti:main

Conversation

@Cristianetaniguti
Copy link
Copy Markdown
Collaborator

@Cristianetaniguti Cristianetaniguti commented Jan 6, 2025

CITATION file added in inst/
With that, the R function citation("BIGapp") works (update when published)
 
Convert to VCF

  • Remove defunct code for action button
  • Add missing input alert in the download button
     
    Dosage Calling
  • Add "Required" * symbol in all required inputs
  • I couldn't make the disable/enable for the download button work, therefore I changed it to a dynamic UI. Now, the download button will only appear if the analysis was run and met all requirements.
     
    VCF filtering
  • Same modification replacing the disable/enable button system by the renderUI
  • Highlight the "save image" button by adding text to icon
     
    Genomic Diversity
  • Round the table number for maximum 4 decimal numbers
  • Highlight the "save image" button by adding text to icon

PCA

  • BUG when group information was absent - exceptions added
  • Highlight the "save image" button by adding text to icon
  • There was a BUG if "run analysis" only with vcf file, and later adding the passport. The graphics were being updated without updating the dataset leading to an error. I added the new variable "pca_data$group_info" to make all variables in the graphics dependent of the data evaluation

DAPC

  • Highlight the "save image" button by adding text to icon
  • Remove nonfunctional code - STRUCTURE plot

Predictive Ability

  • Add "required" symbol * to inputs
  • Highlight the "save image" button by adding text to icon

 Genomic Prediction

  • Highlight the "save image" button by adding text to icon

@Cristianetaniguti Cristianetaniguti changed the title Review tabs 1-4 Review Jan 6, 2025
@Cristianetaniguti Cristianetaniguti added the work in progress More commits are coming label Jan 6, 2025
@Cristianetaniguti Cristianetaniguti removed the work in progress More commits are coming label Jan 6, 2025
Comment thread R/mod_Filtering.R
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Updated save button to show "Save" since some modules allow both file and image saving

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

General comment for later to-dos. We could update the UI to maybe change the Download VCF file to be positioned better, and maybe adjust the color to be more visible? ie a light purple

Copy link
Copy Markdown
Collaborator

@alex-sandercock alex-sandercock left a comment

Choose a reason for hiding this comment

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

Overall looks nice, though there are a few bugs that need to be fixed:

  • PCA plot does not update when the user changes which column to use to color the points
  • The manhattan plot x-axis changes only apply to the "all" figure

Comment thread R/mod_PCA.R
Copy link
Copy Markdown
Collaborator

@alex-sandercock alex-sandercock Jan 29, 2025

Choose a reason for hiding this comment

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

  • a bug has been introduced in the PCA module:
    • When the user updates the "Color Variable" column selected, the plot does not update as intended

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed! But I noticed that the 3D graphics don't update with the "Color Category" selection. I added a tag "2D-Plot only" to the button label. Is that good?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That's perfect for now since the users can remove samples within the 3D plotly window

Comment thread R/mod_dosage2vcf.R
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • We will need to generalize the warning message and adapt the warning message to trigger depending on what time of file format the user is trying to convert.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

^ Future update

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Next pull request has it! 🚀

Comment thread R/mod_gwas.R
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • The update to the manhattan plot x-axis labels only applies to the "all" model selection figure, and not the other specific models

@alex-sandercock alex-sandercock merged commit 7399415 into Breeding-Insight:cris_bridge Jan 30, 2025
Copy link
Copy Markdown
Collaborator

@alex-sandercock alex-sandercock left a comment

Choose a reason for hiding this comment

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

Looks great!

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.

2 participants