Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a3d8d46
Adding new terms re: data retention, embargoes
bistline Jan 8, 2025
2457094
Adding validations for data embargoes
bistline Jan 8, 2025
f332fc9
Adding test
bistline Jan 9, 2025
999552a
Enforce embargo only on public studies
bistline Jan 9, 2025
6f600f3
Updating ToS language
bistline Jan 9, 2025
7005976
Updating ToS language, adding callout to notify user that terms have …
bistline Jan 9, 2025
b0a57a6
Use lists instead of select menu, for glanceability
eweitz Jan 16, 2025
95fb552
Widen DE selection panel, given pairwise default
eweitz Jan 16, 2025
30eb8f5
Componentize GroupListMenu for DE selection
eweitz Jan 16, 2025
03588d2
Add special "Rest" group to menu B
eweitz Jan 16, 2025
ff217c6
Ensure only 1 checkbox can be selected in each DE group menu
eweitz Jan 16, 2025
987b1d1
Use radio, not checkbox; style menus
eweitz Jan 22, 2025
8d0aa9c
Add visuals for available vs not-yet-available DE groups
eweitz Jan 23, 2025
421ae0b
Show DE availability note on hover
eweitz Jan 23, 2025
1dde317
Improve choice indication
eweitz Jan 23, 2025
31d1ffb
Refine note logic in pairwise DE picker
eweitz Jan 23, 2025
0218f01
Enforce picking from menu A first in pairwise DE select UI
eweitz Jan 24, 2025
bc37aa5
Hover all overs upon hovering "rest", change blue -> yellow
eweitz Jan 27, 2025
5047678
Add feature flag for default pairwise DE UI
eweitz Jan 27, 2025
7977bea
Integrate flag for default pairwise DE UI
eweitz Jan 27, 2025
1b92e21
Widen pairwise DE picker; highlight item on hover
eweitz Jan 27, 2025
3320e5c
Use wide DE picker only when pairwise is default
eweitz Jan 28, 2025
8edf64b
Clarify "rest" via hover text, improve accessibility per demo
eweitz Jan 29, 2025
a9574f9
Add test for updated DE picker, simplify code
eweitz Jan 29, 2025
fb8566f
amending terms re: legal review
bistline Jan 29, 2025
b657213
Shorten note text, for easier responsiveness
eweitz Jan 29, 2025
c7cceaa
Test DE pairwise menu B is disabled at start
eweitz Jan 29, 2025
de1ffcd
Add ticketed TODOs
eweitz Jan 29, 2025
6ca4b6f
Refine feature flag down migration, per review
eweitz Jan 30, 2025
a8bd492
Merge pull request #2195 from broadinstitute/jb-data-retention-tos
bistline Feb 10, 2025
cf34ad5
Bump net-imap from 0.5.5 to 0.5.6
dependabot[bot] Feb 10, 2025
ce0f825
Refine note text
eweitz Feb 11, 2025
27dcae4
Merge pull request #2197 from broadinstitute/ew-pairwise-de-ui
eweitz Feb 11, 2025
61a2dbf
Merge pull request #2199 from broadinstitute/dependabot/bundler/net-i…
bistline Feb 11, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ GEM
multi_xml (0.6.0)
multipart-post (2.3.0)
naturally (2.2.1)
net-imap (0.5.5)
net-imap (0.5.6)
date
net-protocol
net-pop (0.1.2)
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ class ApplicationController < ActionController::Base
before_action :get_download_quota
before_action :get_deployment_notification
before_action :set_selected_branding_group
before_action :check_tos_acceptance
# allow users to view privacy policy even if they haven't accepted the ToS yet
before_action :check_tos_acceptance, except: :privacy_policy
before_action :set_ab_test_assignments

rescue_from ActionController::InvalidAuthenticityToken, with: :invalid_csrf
Expand Down
6 changes: 5 additions & 1 deletion app/controllers/profiles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ def update_firecloud_profile
end
end

def accept_tos; end
def accept_tos
@previous_acceptance = TosAcceptance.where(
email: @user.email, :version.ne => TosAcceptance::CURRENT_VERSION
).order_by(&:version).last
end

def record_tos_action
user_accepted = tos_params[:action] == 'accept'
Expand Down
33 changes: 29 additions & 4 deletions app/javascript/components/explore/DifferentialExpressionPanel.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
getCanonicalSize, getCanonicalSignificance
} from '~/lib/validation/validate-differential-expression'
import { contactUsLink } from '~/lib/error-utils'

import { getFeatureFlagsWithDefaults } from '~/providers/UserProvider'

import {
createColumnHelper,
Expand All @@ -25,7 +25,8 @@ import {

import DifferentialExpressionModal from '~/components/explore/DifferentialExpressionModal'
import {
OneVsRestDifferentialExpressionGroupPicker, PairwiseDifferentialExpressionGroupPicker
OneVsRestDifferentialExpressionGroupPicker, PairwiseDifferentialExpressionGroupPicker,
PairwiseDifferentialExpressionGroupLists
} from '~/components/visualization/controls/DifferentialExpressionGroupPicker'

import {
Expand Down Expand Up @@ -725,9 +726,12 @@ export default function DifferentialExpressionPanel({
setUnfoundGenes(unfoundNames)
}, [deGenes, searchedGenes, findMode])

const flags = getFeatureFlagsWithDefaults()
const isPairwiseUi = flags?.default_pairwise_de_ui === true

return (
<>
{!hasPairwiseDe &&
{!isPairwiseUi && !hasPairwiseDe &&
<OneVsRestDifferentialExpressionGroupPicker
bucketId={bucketId}
clusterName={clusterName}
Expand All @@ -745,7 +749,7 @@ export default function DifferentialExpressionPanel({
significanceMetric={significanceMetric}
/>
}
{hasPairwiseDe &&
{!isPairwiseUi && hasPairwiseDe &&
<PairwiseDifferentialExpressionGroupPicker
bucketId={bucketId}
clusterName={clusterName}
Expand All @@ -766,6 +770,27 @@ export default function DifferentialExpressionPanel({
/>
}

{isPairwiseUi &&
<PairwiseDifferentialExpressionGroupLists
bucketId={bucketId}
clusterName={clusterName}
annotation={annotation}
setShowDeGroupPicker={setShowDeGroupPicker}
deGenes={deGenes}
setDeGenes={setDeGenes}
deGroup={deGroup}
setDeGroup={setDeGroup}
countsByLabelForDe={countsByLabelForDe}
deObjects={deObjects}
setDeFilePath={setDeFilePath}
deGroupB={deGroupB}
setDeGroupB={setDeGroupB}
hasOneVsRestDe={hasOneVsRestDe}
sizeMetric={sizeMetric}
significanceMetric={significanceMetric}
/>
}

{
(
(!hasPairwiseDe && genesToShow) ||
Expand Down
30 changes: 23 additions & 7 deletions app/javascript/components/explore/ExploreDisplayTabs.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,9 @@ export default function ExploreDisplayTabs({
studyAccession, exploreInfo, setExploreInfo, exploreParams, updateExploreParams,
clearExploreParams, exploreParamsWithDefaults, routerLocation
}) {
const flags = getFeatureFlagsWithDefaults()
const hasDefaultPairwiseDeUi = flags?.default_pairwise_de_ui === true

const [, setRenderForcer] = useState({})
const [dataCache] = useState(createCache())
// tracks whether the view options controls are open or closed
Expand All @@ -250,7 +253,10 @@ export default function ExploreDisplayTabs({
const [, setShowDeGroupPicker] = useState(false)
const [deGenes, setDeGenes] = useState(null)
const [showDifferentialExpressionPanel, setShowDifferentialExpressionPanel] = useState(deGenes !== null)
const [showUpstreamDifferentialExpressionPanel, setShowUpstreamDifferentialExpressionPanel] = useState(deGenes !== null)

const [
showUpstreamDifferentialExpressionPanel, setShowUpstreamDifferentialExpressionPanel
] = useState(deGenes !== null)

let initialPanel = 'options'
if (showDifferentialExpressionPanel || showUpstreamDifferentialExpressionPanel) {
Expand All @@ -262,6 +268,10 @@ export default function ExploreDisplayTabs({

// Hash of trace label names to the number of points in that trace
const [countsByLabelForDe, setCountsByLabelForDe] = useState(null)
const showDifferentialExpressionPicker = (
hasDefaultPairwiseDeUi &&
showViewOptionsControls && initialPanel === 'differential-expression' && deGenes === null
)
const showDifferentialExpressionTable = (showViewOptionsControls && deGenes !== null)
const plotContainerClass = 'explore-plot-tab-content'

Expand Down Expand Up @@ -478,12 +488,16 @@ export default function ExploreDisplayTabs({
let side
if (showViewOptionsControls) {
if (
(deGenes !== null) ||
(hasPairwiseDe && (showDifferentialExpressionPanel || showUpstreamDifferentialExpressionPanel))
panelToShow === 'differential-expression'
) {
// DE table is shown, or pairwise DE is available. Least horizontal space for plots.
main = 'col-md-9'
side = 'col-md-3 right-panel'
if (hasDefaultPairwiseDeUi && deGenes === null) {
main = 'col-md-8'
side = 'col-md-4 right-panel'
} else {
// DE table is shown
main = 'col-md-9'
side = 'col-md-3 right-panel'
}
} else if (panelToShow === 'cell-filtering') {
main = 'col-md-10-5'
side = 'col-md-2-5 right-panel'
Expand Down Expand Up @@ -604,6 +618,7 @@ export default function ExploreDisplayTabs({
plotPointsSelected,
showRelatedGenesIdeogram,
showViewOptionsControls,
showDifferentialExpressionPicker,
showDifferentialExpressionTable,
scatterColor: exploreParamsWithDefaults.scatterColor,
setCountsByLabelForDe,
Expand All @@ -619,7 +634,8 @@ export default function ExploreDisplayTabs({
studyAccession={studyAccession}
updateDistributionPlot={distributionPlot => updateExploreParams({ distributionPlot }, false)}
dimensions={getPlotDimensions({
showRelatedGenesIdeogram, showViewOptionsControls, showDifferentialExpressionTable
showRelatedGenesIdeogram, showViewOptionsControls,
showDifferentialExpressionPicker, showDifferentialExpressionTable
})}
cellFaceting={cellFaceting}
filteredCells={filteredCells}
Expand Down
7 changes: 5 additions & 2 deletions app/javascript/components/explore/ScatterTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ const MAX_PLOTS = PLOTLY_CONTEXT_NAMES.length
*/
export default function ScatterTab({
exploreInfo, exploreParamsWithDefaults, updateExploreParamsWithDefaults, studyAccession, isGene, isMultiGene,
plotPointsSelected, isCellSelecting, showRelatedGenesIdeogram, showViewOptionsControls, showDifferentialExpressionTable,
plotPointsSelected, isCellSelecting, showRelatedGenesIdeogram, showViewOptionsControls,
showDifferentialExpressionPicker, showDifferentialExpressionTable,
scatterColor, setCountsByLabelForDe, dataCache, filteredCells, cellFilteringSelection
}) {
// maintain the map of plotly contexts to the params that generated the corresponding visualization
Expand Down Expand Up @@ -61,7 +62,9 @@ export default function ScatterTab({
dimensionProps={{
isMultiRow,
isTwoColumn: isTwoColRow,
showRelatedGenesIdeogram, showViewOptionsControls,
showRelatedGenesIdeogram,
showViewOptionsControls,
showDifferentialExpressionPicker,
showDifferentialExpressionTable
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import { fetchBucketFile } from '~/lib/scp-api'
import PlotUtils from '~/lib/plot'
const { getLegendSortedLabels } = PlotUtils


// https://stackoverflow.com/questions/37909134/nbsp-jsx-not-working
const blankSpace = '\u00A0' // How React JSX does &nbsp;

const basePath = '_scp_internal/differential_expression/'

// Value to show in menu if user has not selected a group for DE
Expand Down Expand Up @@ -43,7 +47,7 @@ export function parseDeFile(tsvText, isAuthorDe=false) {
//
// TODO: There are usually more columns than size (e.g. logfoldchanges)
// and significance (e.g. pvals_adj) that may well be of interest.
// However, we don't parse those here before there is no UI for them.
// However, we don't parse those here because there is no UI for them.
// If we opt to build a UI for further metrics (e.g. pctNzGroup in Scanpy
// / pct.1 in Seurat, etc.), we would need to order them canonically
// across SCP-computed DE results and (Ingest Pipeline-processed) author
Expand Down Expand Up @@ -144,6 +148,174 @@ function getMatchingDeOption(
return matchingDeOption
}

/** List menu of groups available to select for DE comparison */
function GroupListMenu({
groups, selectedGroups, updateSelectedGroups, setNote, isMenuB=false,
hoverAllOthers, setHoverAllOthers
}) {

if (isMenuB) {
groups.unshift('rest')
}

const groupsIndex = isMenuB ? 1 : 0
const otherGroupsIndex = isMenuB ? 0 : 1
const otherMenuSelection = selectedGroups[otherGroupsIndex]

return (
<>
{groups.map((group, i) => {
// If this menu has a selected group and this group isn't it,
// then disable this group
const isInvalid = group === otherMenuSelection && isMenuB

if (isInvalid) {return ''}

const isMenuANull = selectedGroups[0] === null

const isDisabled = isMenuANull && isMenuB
const disabledClass = ''
let noteClass = ''
let noteText = ''
let hoverClass = ''

const isRest = group === 'rest'

// TODO (SCP-5912): Integrate DE pairwise comparison availability
const isAvailable = isRest

if (isMenuB) {
if (!isDisabled) {
if (isAvailable) {
if (isRest) {
noteText = 'All other groups'
} else {
noteText = blankSpace
}
noteClass = 'available'
} else {
noteText = 'Pick for pairwise DE'
noteClass = 'not-yet-available'
}
}

if (isDisabled) {
noteText = 'Select from left menu first'
noteClass = 'disabled'
}

if (hoverAllOthers) {
hoverClass = 'hover'
}
}

let ariaLabel = ''
if (isMenuB) {
ariaLabel = (noteText[0].toUpperCase() + noteText.slice(1)).replaceAll('-', ' ')
}
const labelClass = `de-group-menu-item ${noteClass} ${disabledClass} ${hoverClass}`

const menuName = `pairwise-menu${isMenuB && '-b'}`
const id = `${menuName}-${i}`

return (
<label
htmlFor={id}
className={labelClass}
aria-label={ariaLabel}
onMouseEnter={() => {
if (isMenuB) {
setNote(noteText)
if (isRest) {setHoverAllOthers(true)}
}
}}
onMouseLeave={() => {
if (isMenuB) {
setNote(blankSpace)
if (isRest) {setHoverAllOthers(false)}
}
}}
key={i}
>
<input
id={id}
type="radio"
className="pairwise-menu-input"
name={menuName}
style={{ marginRight: '4px' }}
disabled={isDisabled}
onChange={event => {
// TODO (SCP-5913): Add downstream views for updated DE picker
const radio = event.target
const isChecked = radio.checked
const groupName = radio.parentElement.innerText
const newSelectedGroups = [...selectedGroups]

const newGroup = isChecked ? groupName : null
if (groupsIndex === 0 && newGroup === selectedGroups[1]) {
newSelectedGroups[1] === null
}

newSelectedGroups[groupsIndex] = newGroup

updateSelectedGroups(newSelectedGroups)
}}
></input>
{group}
</label>
)
})}
</>
)
}

/** Pick groups of cells for pairwise differential expression (DE) */
export function PairwiseDifferentialExpressionGroupLists({
deGenes, countsByLabelForDe
}) {
const groups = getLegendSortedLabels(countsByLabelForDe)

const [selectedGroups, setSelectedGroups] = useState([null, null])
const [note, setNote] = useState(blankSpace)
const [hoverAllOthers, setHoverAllOthers] = useState(false)

/** Set new selection for DE groups to compare */
function updateSelectedGroups(newSelectedGroups) {
setSelectedGroups(newSelectedGroups)
}

return (
<>
<div className="differential-expression-picker">
<div className="pairwise-menu">
<p>Pick groups to compare.</p>
<GroupListMenu
groups={groups}
selectedGroups={selectedGroups}
updateSelectedGroups={updateSelectedGroups}
hoverAllOthers={hoverAllOthers}
setHoverAllOthers={setHoverAllOthers}
/>
</div>
<div className="vs-note pairwise-lists">vs. </div>
<div className="pairwise-menu pairwise-menu-b">
<p><i>{note}</i></p>
<GroupListMenu
groups={groups}
selectedGroups={selectedGroups}
updateSelectedGroups={updateSelectedGroups}
setNote={setNote}
isMenuB={true}
hoverAllOthers={hoverAllOthers}
setHoverAllOthers={setHoverAllOthers}
/>
</div>
</div>
{deGenes && <><br/><br/></>}
</>
)
}

/** Pick groups of cells for pairwise differential expression (DE) */
export function PairwiseDifferentialExpressionGroupPicker({
bucketId, clusterName, annotation, deGenes, deGroup, setDeGroup,
Expand Down
Loading
Loading