From eb684f52a5e75d60e9901ab7c69e30ccf11ce7a0 Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Tue, 24 Sep 2024 09:32:31 -0400 Subject: [PATCH 01/23] Validate annotation names have only letters, numbers, underscore --- .../lib/validation/shared-validation.js | 29 +++++++++++++++++++ .../lib/validation/validate-file-content.js | 1 + 2 files changed, 30 insertions(+) diff --git a/app/javascript/lib/validation/shared-validation.js b/app/javascript/lib/validation/shared-validation.js index f4e5d2006..9361e3c7f 100644 --- a/app/javascript/lib/validation/shared-validation.js +++ b/app/javascript/lib/validation/shared-validation.js @@ -231,6 +231,35 @@ export function validateUnique(headers) { return issues } +/** + * Check headers for disallowed characters + */ +export function validateAlphanumericAndUnderscores(headers) { + // eslint-disable-next-line max-len + // Mirrors https://github.com/broadinstitute/scp-ingest-pipeline/blob/7c3ea039683c3df90d6e32f23bf5e6813d8fbaba/ingest/validation/validate_metadata.py#L1223 + const issues = [] + const uniques = new Set(headers) + const prohibitedChars = new RegExp(/[^A-Za-z0-9_]/) + + const problemHeaders = [] + + // Do headers have prohibited characters? + uniques.forEach(header => { + const hasProhibited = (header.search(prohibitedChars) !== -1) + if (hasProhibited) { + problemHeaders.push(header) + } + }) + + if (problemHeaders.length > 0) { + const problems = `"${problemHeaders.join('", "')}"` + const msg = `Update these headers to use only letters, numbers, and underscores: ${problems}` + issues.push(['error', 'format:cap:only-alphanumeric-underscore', msg]) + } + + return issues +} + /** Verifies metadata file has all required columns */ export function validateRequiredMetadataColumns(parsedHeaders, isAnnData=false) { const issues = [] diff --git a/app/javascript/lib/validation/validate-file-content.js b/app/javascript/lib/validation/validate-file-content.js index 338ba0fa2..66a5eef24 100644 --- a/app/javascript/lib/validation/validate-file-content.js +++ b/app/javascript/lib/validation/validate-file-content.js @@ -163,6 +163,7 @@ function validateCapFormat([headers, annotTypes]) { // Check format rules that apply to both metadata and cluster files issues = issues.concat( validateUnique(headers), + validateAlphanumericAndUnderscore(headers), validateNameKeyword(headers), validateTypeKeyword(annotTypes), validateGroupOrNumeric(annotTypes), From 858f04384e6925e56a10f986e658107f5f82c1fd Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Tue, 24 Sep 2024 15:19:29 -0400 Subject: [PATCH 02/23] Add AnnData test file for invalid annotation name (period) --- .../invalid_annotation_name_period.h5ad | Bin 0 -> 51184 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/test_data/invalid_annotation_name_period.h5ad diff --git a/test/test_data/invalid_annotation_name_period.h5ad b/test/test_data/invalid_annotation_name_period.h5ad new file mode 100644 index 0000000000000000000000000000000000000000..be51a8ff6521c8c29276929b7f9347282666a5b8 GIT binary patch literal 51184 zcmeHQU36SWk?ye*M@eiV=g&&`$q*nGkeG4e4bJ{$EL+Y5jIB5hh`*J_(%2&+&4`g~ z%lvEx2tgqP%)j95gC6~`9JD`YR|gI-VfXCnkw-o1(T{%g*}rU6SABO{Ew#p$rUTC0 zKDxKNs;jH3>vmQ3?R!V})xwdZyY6`K4#TOd%iLnRZ4sw8HC_Er!2m}6uog0!&uV^o zn-ns{k?VRC->v+!+Xc~3{Hc@u{l;uF;E!CZ&DC3^bWjnOelLZ*!fE#Q)1}3i<4jPtey6d+qw?<6{LYtZQ)AVNv}(=wR);T}V1_!e{fiJ(?zOBk z+XNwyAuE;S!`sPnnR0jbA*IQhJzZwcOGi7*@)l!B+L=j*ndk z{K>%`!XNX3Yt~E8A3t#@NimKbKuxO;=F3{42r#vCc8(E6>4@9S`r5kv4R|@CSnmXEk5ed7z*{ z{rO_eq9~u>0$Inev9;?z}AWu~Hc=P8TUMOS1My#oWIA z`*M`Se1I?3YQ>9$hd%*6x9@RF4;VS)?B&{$=(G?Tc_r2oF+m#l!d+jCQ{ z=gtu^C0XD!@qIcU?)2!7R;w1XQ}M{BrfcloA+Fh_^}f9CP(0*d<(;V&CrXOHE6@(! z3VItI8<}R(-D~JY?{_O6`mYr)Sj;^E`g;}ssOIBir)$O9#i7YsX|h-=PE&}Xa&@{& z6KZIvTA8knSI=G?8ZVwMjdPRxv>nDvO_oN+N>k=R#iPCA*R+49(!ERbjmFO;po4y=c+ydNw>=w!d2xEII#ryQ z94`%xjdIm{v>x$gv|6dwsO}-f`|~)fc-YtBLxOm(kM9%2!#@77Al~QWj|k%ZKE7WN zAMo)<1@VZFe?|~{eY^%n#Ir>HXNB;CKK;i8@gWa?M92GKAFt!ZdQB*B<_m_WQJt*M9RpUi(!w zC3-%j{i>RpcsIP?Ue_$s9SE(^?;re)|1PS@DpsP@Sk|r;3wfrOFgnjd}I{y#IpM`*!rA;zRO}2in=2 z2C4%;$j@saPQXKcSw$Lye)^A|7|P{xPd$~Bc|R%5f_~$RTJQ6*MQRR!eL3!uJN8Lh{Qher>xiOS44J zkpC+l9{T^P;xYeU=<7du;y7P|-2RVJ(EMAi56S;hfNx##A^E=+X!msw5Bb00;UWLa z9{%)rwaT{lO~r@ge#L7Ce&14jNbcD{yH`CtpYM?bGA9`X$wI%Ow*?!c3F>U_tm z_vg*)0lwc=d`SN91=?Nm@R0xe9v<@lK=C^?r`7uehr}N$jB)vL|0uxk$BGZh{gXhu zpDG^q=if)p1={^g@sJ<)%Rg8AcFl3WY)qY(W_kLfW1bE!yD;N{o2PPJVQ$ATABGOV zsAnA4<>}xu=L~b1k2)O;qkq78!WpeYJ>%+%<2o1SGLCtrW7yEaFdqyV!n zo{xDt7>_c;%*Ti8jHiR^jZ8J2OF1`%6Y&f57WK7a@8w_})T@^ml_mtC0gz|B*+t}ys8j_U!V zo^f24r-RF!Gt6Z^>U1!S{sHF+XS5FWjH@e->s*-2IOdg(VM7PQd@v08e7LUl#-U|; zKIZ9QJjx6+A0MtWo(`@zPMP7n!gb0S=9~{h2g7_Y?8>(uY+PC@=K zXcxZ2gPF{&XWzVy4m?bF#)FS~aK8s1_F%qOb@}nVv1i{yvZev27 z=f|8z^4vWWeD0oOB~$8=EIoIa2L5=@kLwSLs_S;jL+#DyUz>aGZtthn4!DfxY&mNn zCV5`C@$=*DFeQ_H7?Q1@cPn~U`DYZKVP{2Yj-3)E*w+d>8A_n*VkbkXmz~MmD7ErC zb#}&-^im0so;yYZ{_}m<(W2T56oLC&*h>Kwo6O-4vTBD7<+Ib|ZHb@L^ZP#Nx7B+- zJGhljVs-&*{i#?!p3mQ6ducvD$NM*-ewy#!h|6%^zj?Q?$}307w(39a{)di-%h1;n zFP10`N=D~wQrf5>@8hsmBu(z!h&rtcX&}aXDZ8jMy#MCMtF6u>yRW4eo%d*C$bT2{ z4ZTk^uko}&G@K+JnczBDsM2$%YQT?2@K*(IU4}R`&?ZH}{`_^|R_)Km865=2))@A; zoA^S0go8yJ5V^mC)zD_&pLO?P?v5n?J;WF9jS*`LJ9G) zXGY+>Wa;^Bt^xQD%$F-V-dTN69K-%9)CcD4ygrC(XumtnHCxKin~dwdM;cmhVx5ZT zS;(y`QGfCt;_R*yc_HfB>$?cHx5G;R0Kt3cz#@=u{Fw&uJE{1n1K&$!_dfnUg82ab z{Zw@MGjEqa+j2YB^A8c9R_pm*qTz&m^XvJ%uIJmkAL3r80?CQ_GRK?4l;E!n9Jru^ zXIanhz@;$x7Mqk=<+Q+?!ITg`mleLoo8FX`Bs1ApqyV~t!sS`P|3T?iUSCbur4NvM zFA1Jw9;d51PVnpDO>jyF4@!>lFN_Kdx@Cp0oe_My&oTVLvm|83?65j!8{mZNy`bYQ z3tHF1;eFv=-t4E;C0TmzFbyD{!9KD&{z08bPzU`gir&&T2Jb0ZfMD2>M~-H}9P{-m zKTM+3uxEEY*<~J~S!sU|Q5rQpw0q2bb<;$ZKD*4(L&u))C)}x%FK}inAdiJD%KzO| zkF7%d>s3PJ+sTsrewd`WKjZ&Cs=?ud5;zF7)pfBO7e7WUp|44P7)J@$%kNie!)Nj9 z+lZf}CFK`tLF%@thaVEADbYX-zy5LJp7?d0$oTmk<}2nm^3AV9uIRkQOpu{%b>6z` zkWUcH74aADy3&*6NhIZGqzndj~=MIbL@#6P$)B)@}$4;J7yW;n(@)NFW&+rS& z`L%T^rnLs#c<_8{@`S?OxA;tgZC&7JD7YCbwkA&~+_+m#u&oR3_;d@abt9s1pZlD@ zt`+SwN)&IA`Ru2TD9`xW3et@w~)aGxZ9x&>Wqdt_QKje@P zav=vEIq3Zy{h)v3J|5$No*+MR)FTHS^aTAO2Oc@-{T%#4{Q!?~fgbXMA7Io&FQ|v! zARp=>ANYeFdP4tz!5{sh9(;fY48DN)|>z-R}0)B}$kc;w&<{rs0I6fmwAU_Zw=frlP|2OP?MI`j`bV94pikPr2c z12Fgih8!3_VCV<*fWZ$i`a^rjLsRBYD#f@jJxL=>?(5VbA6!h5Q^V(I**`-Sn)f@#z?7Q&o^WI zo{t{Che$WD?*-kAZCCv6Vkf&nf?pVHcDMm`=Tyb;>%*kJ#IHw>-%HWs_h}kW3_Xny zU&QZ0z8R-9qyDO$@q3i`#*n{6e3R?U!c$7@n~j~)^~UesqR_)m1xnAIqJdES9@KdR zJ;aLNSJ@aTHSGCjjNi-AvciI3se z&yp$=zaBk)nd#>o zt)8o2V`Wf+JqeVaJ4FMbbwSR1$+g|<0_Ye2R|ww?0lxWv^W{4u_#O$qgA)4gNvrRi zY*fGIBPt{ezWWmT&I`Wl68esdx?wYefxSifjsH&jMt{#&h`zqyB_sS=o}C`0cC9bw zl%H^2dxk%}!a*oai3VcC<8#D4iD%K*EtApLEf;7!G4#Yf1nZWodVkWbe!89O7QSVQ zA^(?%Z>#mlG80jP{RxzwJ4FMb^+p!44wI;eCxeI5xq|M_HI>8|Q~51{k+L6xj+ z8RGT4{%EP8oSXisUHd(;|6f5p`ugU}q#Nif`xQ}CyVf@|%1_n5CWC)n(*3bD1RaWq z5%>5~ymjlF%hA_2?7Cv;=_TTe^^NaGoBm^S`#J;(bQdzQ= z>!7)>3nA=Oo26NYXdtu>TB@&BK)csLn6Ja%lztl8UDN(6gKDCu8D5)BZLW z|8_DHoiyLE{0109L2*vM$&Lij{R{UQ1 zk@V2ej+*v&v&8RS^@F`omCc*u*qk49{S{vd*!S=crJHue??vUeN_Yo=U+DV2gG`H2 z{5q>R@$1p=zXzhnZ=PB);vc&V#Bcxq|8Ldq-1z*0k-zW}0w|UDOTJb4Z)Y}Y zv_GX{4E@g$-_ZASvOi-1j9HMZ<^I76|8qo2@G}CX=T6apzy9dffiLkSa}wU4RJW*K znQ{nju!wkC|9ffJ(DqIHTmSnxyR%R0T+N#UcbWrlZkLAmf9%R8$=dNf8aAm`?>XM0 zJtE)y|A8OYxR6iAz=ft>Hn*PP`#SvpQ?^XRb*E3nFZJp#jmge^*|Db-Sb<32( z)xa}{PaLy0rG^Ll(rIm;J2zv=w`n8b`-=S5zy6bNe3Omm_!w6H~HrjPlXqbn#5BI8m}#{r(e`{dR~Sz@sDZ0UrI_q5Oac zy^a6t@W(hXuFs)1AksRmLF{7Gn_o%_Ss-{E^| z?9b2gW;dnFx}UezeI&l0M?bt*5JUdkh;N4O+<7mghYn8IuUh#hfg6&w+~4m02O-4$ zy*5j;4$*+Wzm55H`IoB|FgP?lRVmRv*<^8gtT=Ax9#iC3lU+BocE7mSw#~@rYLloP7!Go#lWX4*sL`HYgdLq1gnns(}CVCu5EX(6ByXqv z@jDJN?2q4(2-)BAzsWcnlBM>idPuYNH4w7D**8|xx7+@}9s3`(;suMUo}RM#d3`^N z;0Z<$Y;%Gq6}I_!@gn_@%9wK%X$LvM*D`CvN%miErTrU8m-Iz1^k=eyUsU{n(kpnB+{C($~(#`979d!QsJr6qdYYCm_=alR%&v4&S_?>iJenawh z`dxlkEQa6Zcg^JYXzBgDT($J Date: Wed, 25 Sep 2024 10:01:33 -0400 Subject: [PATCH 03/23] Update tests to point to clearer-named files --- .../lib/validation/validate-anndata.js | 2 +- test/js/lib/validate-anndata.test.js | 40 +++++++++++++------ 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/app/javascript/lib/validation/validate-anndata.js b/app/javascript/lib/validation/validate-anndata.js index d05fbe7bd..9d55a54c1 100644 --- a/app/javascript/lib/validation/validate-anndata.js +++ b/app/javascript/lib/validation/validate-anndata.js @@ -1,4 +1,4 @@ -import {openH5File} from 'hdf5-indexed-reader' +import { openH5File } from 'hdf5-indexed-reader' import { validateUnique, validateRequiredMetadataColumns, diff --git a/test/js/lib/validate-anndata.test.js b/test/js/lib/validate-anndata.test.js index ec9e0a5ad..d5b9890ae 100644 --- a/test/js/lib/validate-anndata.test.js +++ b/test/js/lib/validate-anndata.test.js @@ -2,10 +2,11 @@ import { getHdf5File, parseAnnDataFile, getAnnDataHeaders, checkOntologyIdFormat } from 'lib/validation/validate-anndata' +const BASE_URL = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata' + describe('Client-side file validation for AnnData', () => { it('Parses AnnData headers', async () => { - // eslint-disable-next-line max-len - const url = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata_test.h5ad' + const url = `${BASE_URL}/valid.h5ad` const expectedHeaders = [ '_index', 'biosample_id', @@ -20,22 +21,20 @@ describe('Client-side file validation for AnnData', () => { 'species', 'species__ontology_label' ] - const remoteProps = {url} + const remoteProps = { url } const hdf5File = await getHdf5File(url, remoteProps) const headers = await getAnnDataHeaders(hdf5File) expect(headers).toEqual(expectedHeaders) }) it('Reports AnnData with valid headers as valid', async () => { - // eslint-disable-next-line max-len - const url = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata_test.h5ad' + const url = `${BASE_URL}/valid.h5ad` const parseResults = await parseAnnDataFile(url) expect(parseResults.issues).toHaveLength(0) }) it('Reports AnnData with invalid headers as invalid', async () => { - // eslint-disable-next-line max-len - const url = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata_test_bad_header_no_species.h5ad' + const url = `${BASE_URL}/invalid_header_no_species.h5ad` const parseResults = await parseAnnDataFile(url) expect(parseResults.issues).toHaveLength(1) @@ -57,10 +56,24 @@ describe('Client-side file validation for AnnData', () => { expect(issues).toHaveLength(0) }) - // TODO: Uncomment this after row-level AnnData parsing PR is merged - // it('Parses AnnData rows and reports invalid ontology IDs', async () => { - // // eslint-disable-next-line max-len - // const url = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata_test_invalid_disease.h5ad' + it('Parses AnnData rows and reports invalid ontology IDs', async () => { + const url = `${BASE_URL}/invalid_disease_id.h5ad` + const parseResults = await parseAnnDataFile(url) + + expect(parseResults.issues).toHaveLength(1) + + const expectedIssue = [ + 'error', + 'ontology:label-lookup-error', + 'Ontology ID "FOO_0000042" is not among accepted ontologies (MONDO, PATO) for key "disease"' + ] + expect(parseResults.issues[0]).toEqual(expectedIssue) + }) + + // TODO (SCP-5813): Uncomment this test upon completing "Enable ontology validation for remote AnnData" + // + // it('Parses AnnData rows and reports invalid ontology labels', async () => { + // const url = `${BASE_URL}/invalid_disease_label.h5ad` // const parseResults = await parseAnnDataFile(url) // expect(parseResults.issues).toHaveLength(1) @@ -68,7 +81,10 @@ describe('Client-side file validation for AnnData', () => { // const expectedIssue = [ // 'error', // 'ontology:label-lookup-error', - // 'Ontology ID "FOO_0000042" is not among accepted ontologies (MONDO, PATO) for key "disease"' + // 'Invalid disease label "foo". Valid labels for MONDO_0018076: tuberculosis, tuberculosis disease, active tuberculosis, Kochs disease, TB', + // { + // 'subtype': 'ontology:invalid-label' + // } // ] // expect(parseResults.issues[0]).toEqual(expectedIssue) // }) From 7e7c585c886d12c58c02ace415231d0b41632ca8 Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Wed, 25 Sep 2024 12:14:31 -0400 Subject: [PATCH 04/23] Remove hyphen from old valid example, per slightly less-old rule Context: https://broadinstitute.slack.com/archives/CBEHTH601/p1727279828415709 --- test/test_data/metadata_example.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_data/metadata_example.txt b/test/test_data/metadata_example.txt index 07270de1b..e5e5b478f 100644 --- a/test/test_data/metadata_example.txt +++ b/test/test_data/metadata_example.txt @@ -1,4 +1,4 @@ -NAME Cluster Sub-Cluster Average Intensity +NAME Cluster Subcluster Average Intensity TYPE group group numeric CELL_0001 CLST_A CLST_A_1 7.742 CELL_0002 CLST_A CLST_A_1 -13.745 @@ -14,4 +14,4 @@ CELL_00011 CLST_C CLST_C_1 0.638 CELL_00012 CLST_C CLST_C_1 8.888 CELL_00013 CLST_C CLST_C_1 -2.27 CELL_00014 CLST_C CLST_C_2 -2.606 -CELL_00015 CLST_C CLST_C_2 -9.089 \ No newline at end of file +CELL_00015 CLST_C CLST_C_2 -9.089 From 7d7fd96e91afb3a5112fc7441d08430654e30bc7 Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Wed, 25 Sep 2024 12:15:35 -0400 Subject: [PATCH 05/23] Remove space from old valid example, per slightly less-old rule Context: https://broadinstitute.slack.com/archives/CBEHTH601/p1727279828415709 --- test/test_data/metadata_example.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_data/metadata_example.txt b/test/test_data/metadata_example.txt index e5e5b478f..0a9e67673 100644 --- a/test/test_data/metadata_example.txt +++ b/test/test_data/metadata_example.txt @@ -1,4 +1,4 @@ -NAME Cluster Subcluster Average Intensity +NAME Cluster Subcluster Average_Intensity TYPE group group numeric CELL_0001 CLST_A CLST_A_1 7.742 CELL_0002 CLST_A CLST_A_1 -13.745 From c12b3756b4d098c2f70c50b688358dcf1430fc7a Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Wed, 25 Sep 2024 16:32:46 -0400 Subject: [PATCH 06/23] Finish alphanumeric + underscore validation, add test data --- .../lib/validation/validate-anndata.js | 2 ++ .../anndata/invalid_annotation_name_period.h5ad | Bin 0 -> 53248 bytes .../metadata_invalid_annotation_name_period.tsv | 12 ++++++++++++ 3 files changed, 14 insertions(+) create mode 100644 test/test_data/anndata/invalid_annotation_name_period.h5ad create mode 100644 test/test_data/metadata_invalid_annotation_name_period.tsv diff --git a/app/javascript/lib/validation/validate-anndata.js b/app/javascript/lib/validation/validate-anndata.js index b8d87e61f..08bc1da94 100644 --- a/app/javascript/lib/validation/validate-anndata.js +++ b/app/javascript/lib/validation/validate-anndata.js @@ -3,6 +3,7 @@ import { openH5File } from 'hdf5-indexed-reader' import { getOAuthToken } from '~/lib/scp-api' import { validateUnique, validateRequiredMetadataColumns, + validateAlphanumericAndUnderscores, metadataSchema, REQUIRED_CONVENTION_COLUMNS } from './shared-validation' import { getAcceptedOntologies, fetchOntologies } from './ontology-validation' @@ -285,6 +286,7 @@ export async function parseAnnDataFile(fileOrUrl, remoteProps) { issues = issues.concat( validateUnique(headers), + validateAlphanumericAndUnderscores(headers), requiredMetadataIssues, ontologyIdFormatIssues, ontologyLabelAndIdIssues diff --git a/test/test_data/anndata/invalid_annotation_name_period.h5ad b/test/test_data/anndata/invalid_annotation_name_period.h5ad new file mode 100644 index 0000000000000000000000000000000000000000..a2f02e0a191ab8ab0679cc51a7fc6598b8df768b GIT binary patch literal 53248 zcmeHQU36SWk?ye_M>saI6DL;whXBa{u^A`c;Ot+-4W5F%h7^r(l0qusL{ygz4m4tsVnyL)!?s7F8QQI9_Rm#ylm@7}8(wa3;> zo1@I_qkFrny1Kf$ZdX;`zISwA9XND&+m?H`7*0JsW|P@&i#WZl$-E_)2v&kqnrAeh z(R_Y`6f(q->v|QxUHNA>3ZkL-lPCK7joDzpAGubW@+>5R35I3jFr#qp1M#i zQO!M>jBUx*B(~9`^foiMawaHSztz~`QF+Hoe&dee!mvjzePaeDUsq2c4cgUP za(wJU;7|5%7XFwQT(eqw{^+qo8N*Ubx}tt)HCo0EXhr_FQcyLTL4~Jv{=vUxKfID| zK;hB7f`@*O|7s=Ow8FE$7Cii1zOj;SP~qv{2>$nV4l!SO4*Kl))E*gWLkA3hfa{%A z{G!eS1r6%Y6>Ann`2-hm9ieB?x5`6|n?=r$M_`#oRRGNE?T-rLl;UIi*+zb%*9=$2 z&rXzgS85|AJHQ>;r*&=@%)Z;qzPELLXUHF|mYz9y?1-&Va{-n0Pt>4TE{_zaiWHe8 zS^J}6e$U=LdCFlvz!z(^;swIPp8%iV^SGr4j2!g1Y`}vaa-e=_tUOYhj$CKde_!!S zR={`Jb1c_$XNj1SEO26cpU#Kd9QwnRip6YIJo3q@8hdw$Yqn{?$QFV>0^ zCB@(FX$Nlwy^V|wPci9^CVJ8P9g2tkYsK>xbEk)Xr{W*ee0=P5tysG-RIQb&#aeNS zLJXBFQx%#}LqnDFRAsz!=EBf;@pNgNo7|=CFy2b-Op$}q-HJ!O#jnC(m(t;Sj*Lx~ zijyS-34?mYH*vT}+e5!&<#WaHv5`HarJ|kr_d5J1tEJ(w(xkah@!+rcCjR#;9rA1G zhe;n$dgy8oT!eMhQ>y?>LIO1{28f~D>bUiD&C#v?@>JL@8AJJ zJnZ881o2)Me^3zbbMc1+@qQQIFNjB6{9!?;JBs|l!Dj@IxE9GdAovfse4Z7=2VML} z1@R#Ve@MrRc?WC_FHiA+OMi3((?iBSJe^YwO>_7jMsiu9Wh=Js*V`12o)>FD`OQa z#;YJIR*YA{RV)L4;KmUZk5;6T5cfW&Fz^UGM;tupKd$&&H5b3K*Bn*Y*Z(mOzvGHW zd>k%LmCjUZW5dO9Tl1W!-3bQ|xu19Nko%nJmTrm zgMH7QEKbuZ;Hf9`q-WXX88L!{t zdjwAC4}4`6@0b6}(yXc7R~$U#|EhyWKYylp%>Nhq`cE7?%9kL&_ahWcvA^Ndf8E3P z>x%d3{~Mlm-*oVh|62|o^1tlhPmfnBY?H4j-Y56Bop#_iqj;a(uX@^j$H7DH?>cz& z^F75wzT*c@+KHdv_oSUVuQ~PZy#Kz3?++C3lmDWp-47i+aa|JTGLAW=W7yEaFdqy< zJ{PWQy-CnAJs)#)Fdk)wnU4$C8BYh-n?#x6g2Hvm8Rnc1LkGirFr1WcHP|F+saya| z@-Y3}HDIm-y-_;A$m?jYIq2qqPh4-tL0+^2eL?A&h7aHYyX_cengSh6!{sE*c;M!! zTvr%;7{~R1QO`K8E6~AZ&Kc%1A9XqyM*n~dgfm))ddAfi$8|}V%Q)tgj$uOw!+bCd z`CPcJ^(H~f^nA?G!FZGzW^&f!EjQ()nJpPrE&rA zx}JX<4W%AF$(kbxZy{U~X0jyAYo85tp#6XYKjXlBPno3WThJuj@4##r$@+o=^DSnw zzR!UVIxz1wBhj`&VfJRz#nwr4>|B-4$ON|$?@!QVBUL5*7KfI66OU^66QUo zB>aQ}V^4|DEt8G=I?u6Jz>Va&d&c|RJ zJ$JYFn`#GK#&fp3E{q4!(KVkRZ-mP?*@vSBs}{B^c1HPU6rN`1MQN6u6eZZ#6?Qt5 zK-a@ghfoUZl;~i2I?9W{X zwrhVj&gdXGwuZ339mE&%o2)40k`0L5-+)D|_5Io8KFl4&DTMrY5?{QRhxIw)-4*+^ z!kDUF@K*1ypVFIk#K+vU=%XN6dVYs%0RH2Y&X=n?-dTM>9Af@5Ys}Y$Ur2`y?RTr` zv65uyO~%#UBlWE}u};NvF67pgs6Tm#`be%5A0#+wue>m|x5G-$H~PEiz#9j3)*{@_5H25H0C&OVqPOg{Nl(|CZ7%zqXRDS05zrzbJT)fj04`I3@7QDLk-W@Jo`Jg*T)Cx{Oui+$|dL#qV5Ur3OO9?*=<9N)3Bv zdmpeRzMonj@w;$PM9=96B#XzAvwNkYx0O9`5fqcgl%0&jt}qhw{Ep^ z-{|G*X`nPoy?pNm+S%=HHJ;aRpk4C1|BTx4{@LuG3j@}565is#++DJsb^L$&4^3xg{kOPk# z^lpxR&_8k)kMTfHkRLhfk%JC;g8q;Lj~w)F4t~CVfXBE%5Bb3lFzTTf)I)EO5A~1_ z{6P;rp?|>OkN!{(KEMM8U%+m^kPH1?E!;7VDt}upaRj z4vZf#^aFap;0GA}p*`dgFW32Vi2Ks#NzCZJP7N#E{rl)a(Mq`c_ZO&x5PJFq@x}eS z`+T6?b$4<-^T|N+^V&VS&Q#_Ut?zl$fYPXBE%)Cq@n!%etkVTb&)uQ{-~Ia%Z*nD4 zsQdT2-qbX-uh{<9%l-Sb`oUa6TgiQh_V3?6MVw*ZhMhd6PTju`DnH@ce1>0`SHIFs z(VdtOap}{f%E+$=kKc2_J^yi;f0xx?#r|V$c9)FbpARJe5b=$! zvxpztw;J~K#_wr%4wPW00;T6}(SR?0&-3ClkwV4q0vkD{hCL_a-g=4O3&G>}Dbfw% zcY~cgrB21~-WHp?@C&1Hhay0ipF_l@BIz{p>%rsqV(|DqOydcmrxD_d_&v%u`IKhV zUv)Bmmxyl&`ClZy(RF6wDMj|p!JX3e#_#<+X(+)?1xnA|q5)t09@TjSJ%ozihK-R@ z!=Be;{9XzkzxkD0*mwQ3C@K!8In~_tWlitDDL-Fp_=VglA4PZKL-=*}*^yrl9>2FA zk}DT(|NINoK?ps)M0^pyb9^&SX+Zr|C*$`x@eLvW1o4foGYd}%_MPWRKuNQf`{(oe zvIFc?TBql3(SR?0=bYbZgo@v9O^K!(s_@wUI(7fO`tLld7oB5AFt2N?_b%FbPwRY_zxi3E%NPc#u(RHRWC)ijJRG{~5w}LV%C|PriIt1m7RQcTarZIcfD>lWWyCeKduH z!FOPM-+jS%UVPtmalhlDZaG@FW4xWtZ~S-KH~M?N!QktQv!oRGwd3qeDRpXnF|YiD zYx5cY@M^WWuA4C-;_*3BXcW(auUqQD*DcdDo)CKCTN$id3~vrl%Br94|tSgh1!MH?&{r3i><}bnf%Xg3=k?_XM504+^qQ z<%rh{UlooGWgPpbPVM)E{(l7*gRgJCLb`#z@(fVw)cR&d`3cwNGyLnK?vFK7bSEZ6 z++$O?dFz`?!Phrmqw$2$Q=Rx?edGGk*ng~T-!i!`^YuXTe}ni&*O|(kBKzh}DUC|j zave0!%qYQ51xnA|q52dyx`Doy8KBgu z_&x9~;paQUFU+c6@zHe~K15vNOA+~XT}X%fz2b84`27lvCxo89O?(l*-TRx^f3zRJ zX9CIpYVtZ$nNwun+$p6|$y&zm(eDT$>{OejS(j+Q7r$q99zhME;&=Wv>7k(=#rC&e z;MRW&5tDCUtkl3eJ^}h_;)IP_q6yYD)0+~>~zu;X&^*g;!At;;&)Ex zZMgR(*|bCG>HEYN@!S1B!|mE#^8Wh=f#koKyv|hS6xladQW}-4W&Fwt&it1GSzO4LI{Y@GCLN5!E zrbq)J;u5>!n-{-N1&`ls+9CAx2JuDwcK`o>>_67FZcfJUp9GTs&E$2aGN;JCX9d3A z_`UQ~p@*Fcl%Bgq1HSm(tMdqY2o}HpLV9RuN3s2_m-t-`9>4igz`pZ87e#d{e$OaB zRew_kzp&`chbF=zO|F3^GSp3HSr)3#2Z)erc<9VCa5nMZa z=HRg-cC5NhfN^itW|LF3vGSSS#agXs-_Y2g_**p3?|FPrp2`~)w(*(D`0mJXaopBy z(t5WYVE*5^oJNXM#TRSEiIRY3v*w^ff44gIcz+xA1bO<3@**Gj1OMFFY0F`Y@&UfD zC_hBNIDkJ~kry1H7v!#fwt3{W|3{hXAcs57+db{{vdDM1hz1v8hlls)1AksRmLFq#8&ykZK^+K&pXtrh!iGvtVC>_Xx26?|!d0-hW*C z{y+NRd-D+T^Sv?&S$@aLT9I@`{f>fZcAXjs5g&Mu8u4xRmmqo_7fOd9OTUHmw@pmj6!1(~zuXe9Qig5W=r%voz}x4Y=_Q>!nM-Ua5e= zp{dDo$(YG%acYd7X(7%PR+3#e)EqBzwrxf}S08Ia@7{}#6-oTxyfq`1f9&M^F#q>2V<1%wd0ve=m%#{)6P5v_F0q zB82_%yA(eATmFrVqaj&pf2xNxTU`S_`szYwEcnmpcZPy^R`quJ!$iE`kpbt6O16(<^)$2w)uGRLa9bg z&X%PepbhE-{p6nL-<{OXIlO*ZhAj| p(Cop-m(<=d4A>?71n_IkVOOqy>7-rpZv#Tu75}#2{{iv0%!>d3 literal 0 HcmV?d00001 diff --git a/test/test_data/metadata_invalid_annotation_name_period.tsv b/test/test_data/metadata_invalid_annotation_name_period.tsv new file mode 100644 index 000000000..b62faed5c --- /dev/null +++ b/test/test_data/metadata_invalid_annotation_name_period.tsv @@ -0,0 +1,12 @@ +NAME donor_id biosample_id sex species species__ontology_label library_preparation_protocol library_preparation_protocol__ontology_label organ organ__ontology_label disease disease__ontology_label invalid.header +TYPE group group group group group group group group group group group group +A donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown +B donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown +C donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown +D donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown +E donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown +F donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown +G donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown +H donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown +I donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown +J donor_1 biosample_1 unknown NCBITaxon_9606 Homo sapiens EFO_0009901 10x 3' v1 UBERON_0000178 blood PATO_0000461 normal unknown From d9e877b67a0db4b95785e42917032275f1b9a102 Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Wed, 25 Sep 2024 17:04:07 -0400 Subject: [PATCH 07/23] Test for disallowed characters, update old test data to conform --- test/js/lib/validate-file-content.test.js | 13 +++++++++++++ test/test_data/validation/header_count_mismatch.tsv | 2 +- .../validation/metadata_bad_has_coordinates.txt | 2 +- .../validation/metadata_bad_type_header.txt | 4 ++-- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/test/js/lib/validate-file-content.test.js b/test/js/lib/validate-file-content.test.js index 0185c972d..36fa32fbf 100644 --- a/test/js/lib/validate-file-content.test.js +++ b/test/js/lib/validate-file-content.test.js @@ -420,6 +420,19 @@ describe('Client-side file validation', () => { const displayedWarning = screen.getByTestId('validation-warning') expect(displayedWarning).toHaveTextContent(issues.warnings[0][2]) }) + + it('Catches disallowed characters in header', async () => { + const file = createMockFile({ + fileName: 'foo.txt', + content: 'NAME,X,Y,invalid.header\nTYPE,numeric,numeric,numeric,numeric\nCELL_0001,34.472,32.211\nCELL_0002,15.975,10.043,5' + }) + const [{ errors }] = await validateLocalFile(file, { file_type: 'Cluster' }) + expect(errors).toHaveLength(1) + + const expectedErrorType = 'format:cap:only-alphanumeric-underscore' + const errorType = errors[0][1] + expect(errorType).toBe(expectedErrorType) + }) }) // With the client side file validation feature flag set to false expect invalid files to pass diff --git a/test/test_data/validation/header_count_mismatch.tsv b/test/test_data/validation/header_count_mismatch.tsv index fc495e025..a2bafbf27 100644 --- a/test/test_data/validation/header_count_mismatch.tsv +++ b/test/test_data/validation/header_count_mismatch.tsv @@ -1,4 +1,4 @@ -NAME Cluster Sub-Cluster Average Intensity +NAME Cluster Subcluster Average_Intensity TYPE group group CELL_0001 CLST_A CLST_A_1 7.742 CELL_0002 CLST_A CLST_A_1 -13.745 diff --git a/test/test_data/validation/metadata_bad_has_coordinates.txt b/test/test_data/validation/metadata_bad_has_coordinates.txt index dd0269fdd..3ee795dc2 100644 --- a/test/test_data/validation/metadata_bad_has_coordinates.txt +++ b/test/test_data/validation/metadata_bad_has_coordinates.txt @@ -1,4 +1,4 @@ -NAME CLUSTER SUB-CLUSTER x +NAME CLUSTER SUBCLUSTER x TYPE group group numeric CELL_0001 CLST_A CLST_A_1 1 CELL_0002 CLST_A CLST_A_1 2 diff --git a/test/test_data/validation/metadata_bad_type_header.txt b/test/test_data/validation/metadata_bad_type_header.txt index 3602d225c..2e1f675ad 100644 --- a/test/test_data/validation/metadata_bad_type_header.txt +++ b/test/test_data/validation/metadata_bad_type_header.txt @@ -1,4 +1,4 @@ -NAME Cluster Sub-Cluster Average Intensity +NAME Cluster Subcluster Average_Intensity notTYPE group group numeric CELL_0001 CLST_A CLST_A_1 7.742 CELL_0002 CLST_A CLST_A_1 -13.745 @@ -14,4 +14,4 @@ CELL_00011 CLST_C CLST_C_1 0.638 CELL_00012 CLST_C CLST_C_1 8.888 CELL_00013 CLST_C CLST_C_1 -2.27 CELL_00014 CLST_C CLST_C_2 -2.606 -CELL_00015 CLST_C CLST_C_2 -9.089 \ No newline at end of file +CELL_00015 CLST_C CLST_C_2 -9.089 From b0290abcf8b58a12b7936d12b7be17be7449d3ac Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Thu, 26 Sep 2024 09:16:17 -0400 Subject: [PATCH 08/23] Refine test data for alphanumeric-underscore header validation --- .../cluster_invalid_annotation_name_period.txt | 17 +++++++++++++++++ .../invalid_annotation_name_period.h5ad | Bin 51184 -> 0 bytes 2 files changed, 17 insertions(+) create mode 100644 test/test_data/cluster_invalid_annotation_name_period.txt delete mode 100644 test/test_data/invalid_annotation_name_period.h5ad diff --git a/test/test_data/cluster_invalid_annotation_name_period.txt b/test/test_data/cluster_invalid_annotation_name_period.txt new file mode 100644 index 000000000..e2dd55efd --- /dev/null +++ b/test/test_data/cluster_invalid_annotation_name_period.txt @@ -0,0 +1,17 @@ +NAME X Y Z Category Intensity invalid.header +TYPE numeric numeric numeric group numeric numeric +CELL_0001 34.472 32.211 60.035 C 0.719 1 +CELL_0002 15.975 10.043 21.424 B 0.904 1 +CELL_0003 -11.688 -53.645 -58.374 A 2.195 1 +CELL_0004 30.04 31.138 33.597 B -1.084 1 +CELL_0005 23.862 33.092 26.904 B 4.256 2 +CELL_0006 -39.07 -14.64 -44.643 A 1.317 2 +CELL_0007 40.039 27.206 55.211 C -4.917 2 +CELL_0008 28.755 27.187 34.686 B -3.777 2 +CELL_0009 -48.601 -13.512 -51.659 A 0.778 2 +CELL_00010 14.653 27.832 28.586 B 4.62 2 +CELL_00011 20.603 32.071 45.484 C -3.019 2 +CELL_00012 -10.333 -51.733 -26.631 A -4.989 2 +CELL_00013 -52.966 -12.484 -60.369 A 3.137 2 +CELL_00014 38.513 26.969 63.654 C -1.74 2 +CELL_00015 12.838 13.047 17.685 B -2.443 2 diff --git a/test/test_data/invalid_annotation_name_period.h5ad b/test/test_data/invalid_annotation_name_period.h5ad deleted file mode 100644 index be51a8ff6521c8c29276929b7f9347282666a5b8..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 51184 zcmeHQU36SWk?ye*M@eiV=g&&`$q*nGkeG4e4bJ{$EL+Y5jIB5hh`*J_(%2&+&4`g~ z%lvEx2tgqP%)j95gC6~`9JD`YR|gI-VfXCnkw-o1(T{%g*}rU6SABO{Ew#p$rUTC0 zKDxKNs;jH3>vmQ3?R!V})xwdZyY6`K4#TOd%iLnRZ4sw8HC_Er!2m}6uog0!&uV^o zn-ns{k?VRC->v+!+Xc~3{Hc@u{l;uF;E!CZ&DC3^bWjnOelLZ*!fE#Q)1}3i<4jPtey6d+qw?<6{LYtZQ)AVNv}(=wR);T}V1_!e{fiJ(?zOBk z+XNwyAuE;S!`sPnnR0jbA*IQhJzZwcOGi7*@)l!B+L=j*ndk z{K>%`!XNX3Yt~E8A3t#@NimKbKuxO;=F3{42r#vCc8(E6>4@9S`r5kv4R|@CSnmXEk5ed7z*{ z{rO_eq9~u>0$Inev9;?z}AWu~Hc=P8TUMOS1My#oWIA z`*M`Se1I?3YQ>9$hd%*6x9@RF4;VS)?B&{$=(G?Tc_r2oF+m#l!d+jCQ{ z=gtu^C0XD!@qIcU?)2!7R;w1XQ}M{BrfcloA+Fh_^}f9CP(0*d<(;V&CrXOHE6@(! z3VItI8<}R(-D~JY?{_O6`mYr)Sj;^E`g;}ssOIBir)$O9#i7YsX|h-=PE&}Xa&@{& z6KZIvTA8knSI=G?8ZVwMjdPRxv>nDvO_oN+N>k=R#iPCA*R+49(!ERbjmFO;po4y=c+ydNw>=w!d2xEII#ryQ z94`%xjdIm{v>x$gv|6dwsO}-f`|~)fc-YtBLxOm(kM9%2!#@77Al~QWj|k%ZKE7WN zAMo)<1@VZFe?|~{eY^%n#Ir>HXNB;CKK;i8@gWa?M92GKAFt!ZdQB*B<_m_WQJt*M9RpUi(!w zC3-%j{i>RpcsIP?Ue_$s9SE(^?;re)|1PS@DpsP@Sk|r;3wfrOFgnjd}I{y#IpM`*!rA;zRO}2in=2 z2C4%;$j@saPQXKcSw$Lye)^A|7|P{xPd$~Bc|R%5f_~$RTJQ6*MQRR!eL3!uJN8Lh{Qher>xiOS44J zkpC+l9{T^P;xYeU=<7du;y7P|-2RVJ(EMAi56S;hfNx##A^E=+X!msw5Bb00;UWLa z9{%)rwaT{lO~r@ge#L7Ce&14jNbcD{yH`CtpYM?bGA9`X$wI%Ow*?!c3F>U_tm z_vg*)0lwc=d`SN91=?Nm@R0xe9v<@lK=C^?r`7uehr}N$jB)vL|0uxk$BGZh{gXhu zpDG^q=if)p1={^g@sJ<)%Rg8AcFl3WY)qY(W_kLfW1bE!yD;N{o2PPJVQ$ATABGOV zsAnA4<>}xu=L~b1k2)O;qkq78!WpeYJ>%+%<2o1SGLCtrW7yEaFdqyV!n zo{xDt7>_c;%*Ti8jHiR^jZ8J2OF1`%6Y&f57WK7a@8w_})T@^ml_mtC0gz|B*+t}ys8j_U!V zo^f24r-RF!Gt6Z^>U1!S{sHF+XS5FWjH@e->s*-2IOdg(VM7PQd@v08e7LUl#-U|; zKIZ9QJjx6+A0MtWo(`@zPMP7n!gb0S=9~{h2g7_Y?8>(uY+PC@=K zXcxZ2gPF{&XWzVy4m?bF#)FS~aK8s1_F%qOb@}nVv1i{yvZev27 z=f|8z^4vWWeD0oOB~$8=EIoIa2L5=@kLwSLs_S;jL+#DyUz>aGZtthn4!DfxY&mNn zCV5`C@$=*DFeQ_H7?Q1@cPn~U`DYZKVP{2Yj-3)E*w+d>8A_n*VkbkXmz~MmD7ErC zb#}&-^im0so;yYZ{_}m<(W2T56oLC&*h>Kwo6O-4vTBD7<+Ib|ZHb@L^ZP#Nx7B+- zJGhljVs-&*{i#?!p3mQ6ducvD$NM*-ewy#!h|6%^zj?Q?$}307w(39a{)di-%h1;n zFP10`N=D~wQrf5>@8hsmBu(z!h&rtcX&}aXDZ8jMy#MCMtF6u>yRW4eo%d*C$bT2{ z4ZTk^uko}&G@K+JnczBDsM2$%YQT?2@K*(IU4}R`&?ZH}{`_^|R_)Km865=2))@A; zoA^S0go8yJ5V^mC)zD_&pLO?P?v5n?J;WF9jS*`LJ9G) zXGY+>Wa;^Bt^xQD%$F-V-dTN69K-%9)CcD4ygrC(XumtnHCxKin~dwdM;cmhVx5ZT zS;(y`QGfCt;_R*yc_HfB>$?cHx5G;R0Kt3cz#@=u{Fw&uJE{1n1K&$!_dfnUg82ab z{Zw@MGjEqa+j2YB^A8c9R_pm*qTz&m^XvJ%uIJmkAL3r80?CQ_GRK?4l;E!n9Jru^ zXIanhz@;$x7Mqk=<+Q+?!ITg`mleLoo8FX`Bs1ApqyV~t!sS`P|3T?iUSCbur4NvM zFA1Jw9;d51PVnpDO>jyF4@!>lFN_Kdx@Cp0oe_My&oTVLvm|83?65j!8{mZNy`bYQ z3tHF1;eFv=-t4E;C0TmzFbyD{!9KD&{z08bPzU`gir&&T2Jb0ZfMD2>M~-H}9P{-m zKTM+3uxEEY*<~J~S!sU|Q5rQpw0q2bb<;$ZKD*4(L&u))C)}x%FK}inAdiJD%KzO| zkF7%d>s3PJ+sTsrewd`WKjZ&Cs=?ud5;zF7)pfBO7e7WUp|44P7)J@$%kNie!)Nj9 z+lZf}CFK`tLF%@thaVEADbYX-zy5LJp7?d0$oTmk<}2nm^3AV9uIRkQOpu{%b>6z` zkWUcH74aADy3&*6NhIZGqzndj~=MIbL@#6P$)B)@}$4;J7yW;n(@)NFW&+rS& z`L%T^rnLs#c<_8{@`S?OxA;tgZC&7JD7YCbwkA&~+_+m#u&oR3_;d@abt9s1pZlD@ zt`+SwN)&IA`Ru2TD9`xW3et@w~)aGxZ9x&>Wqdt_QKje@P zav=vEIq3Zy{h)v3J|5$No*+MR)FTHS^aTAO2Oc@-{T%#4{Q!?~fgbXMA7Io&FQ|v! zARp=>ANYeFdP4tz!5{sh9(;fY48DN)|>z-R}0)B}$kc;w&<{rs0I6fmwAU_Zw=frlP|2OP?MI`j`bV94pikPr2c z12Fgih8!3_VCV<*fWZ$i`a^rjLsRBYD#f@jJxL=>?(5VbA6!h5Q^V(I**`-Sn)f@#z?7Q&o^WI zo{t{Che$WD?*-kAZCCv6Vkf&nf?pVHcDMm`=Tyb;>%*kJ#IHw>-%HWs_h}kW3_Xny zU&QZ0z8R-9qyDO$@q3i`#*n{6e3R?U!c$7@n~j~)^~UesqR_)m1xnAIqJdES9@KdR zJ;aLNSJ@aTHSGCjjNi-AvciI3se z&yp$=zaBk)nd#>o zt)8o2V`Wf+JqeVaJ4FMbbwSR1$+g|<0_Ye2R|ww?0lxWv^W{4u_#O$qgA)4gNvrRi zY*fGIBPt{ezWWmT&I`Wl68esdx?wYefxSifjsH&jMt{#&h`zqyB_sS=o}C`0cC9bw zl%H^2dxk%}!a*oai3VcC<8#D4iD%K*EtApLEf;7!G4#Yf1nZWodVkWbe!89O7QSVQ zA^(?%Z>#mlG80jP{RxzwJ4FMb^+p!44wI;eCxeI5xq|M_HI>8|Q~51{k+L6xj+ z8RGT4{%EP8oSXisUHd(;|6f5p`ugU}q#Nif`xQ}CyVf@|%1_n5CWC)n(*3bD1RaWq z5%>5~ymjlF%hA_2?7Cv;=_TTe^^NaGoBm^S`#J;(bQdzQ= z>!7)>3nA=Oo26NYXdtu>TB@&BK)csLn6Ja%lztl8UDN(6gKDCu8D5)BZLW z|8_DHoiyLE{0109L2*vM$&Lij{R{UQ1 zk@V2ej+*v&v&8RS^@F`omCc*u*qk49{S{vd*!S=crJHue??vUeN_Yo=U+DV2gG`H2 z{5q>R@$1p=zXzhnZ=PB);vc&V#Bcxq|8Ldq-1z*0k-zW}0w|UDOTJb4Z)Y}Y zv_GX{4E@g$-_ZASvOi-1j9HMZ<^I76|8qo2@G}CX=T6apzy9dffiLkSa}wU4RJW*K znQ{nju!wkC|9ffJ(DqIHTmSnxyR%R0T+N#UcbWrlZkLAmf9%R8$=dNf8aAm`?>XM0 zJtE)y|A8OYxR6iAz=ft>Hn*PP`#SvpQ?^XRb*E3nFZJp#jmge^*|Db-Sb<32( z)xa}{PaLy0rG^Ll(rIm;J2zv=w`n8b`-=S5zy6bNe3Omm_!w6H~HrjPlXqbn#5BI8m}#{r(e`{dR~Sz@sDZ0UrI_q5Oac zy^a6t@W(hXuFs)1AksRmLF{7Gn_o%_Ss-{E^| z?9b2gW;dnFx}UezeI&l0M?bt*5JUdkh;N4O+<7mghYn8IuUh#hfg6&w+~4m02O-4$ zy*5j;4$*+Wzm55H`IoB|FgP?lRVmRv*<^8gtT=Ax9#iC3lU+BocE7mSw#~@rYLloP7!Go#lWX4*sL`HYgdLq1gnns(}CVCu5EX(6ByXqv z@jDJN?2q4(2-)BAzsWcnlBM>idPuYNH4w7D**8|xx7+@}9s3`(;suMUo}RM#d3`^N z;0Z<$Y;%Gq6}I_!@gn_@%9wK%X$LvM*D`CvN%miErTrU8m-Iz1^k=eyUsU{n(kpnB+{C($~(#`979d!QsJr6qdYYCm_=alR%&v4&S_?>iJenawh z`dxlkEQa6Zcg^JYXzBgDT($J Date: Thu, 26 Sep 2024 16:13:02 -0400 Subject: [PATCH 09/23] Fixing issues with reporting ingestSummary events --- app/models/differential_expression_result.rb | 2 +- app/models/ingest_job.rb | 34 ++++++++++++++------ test/models/ingest_job_test.rb | 22 +++++++++---- 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/app/models/differential_expression_result.rb b/app/models/differential_expression_result.rb index 04a02a7e0..8810f6374 100644 --- a/app/models/differential_expression_result.rb +++ b/app/models/differential_expression_result.rb @@ -228,7 +228,7 @@ def remove_output_files # in production, DeleteQueueJob will handle all necessary cleanup return true if study.nil? || study.detached || study.queued_for_deletion - identifier = "#{study.accession}:#{annotation_identifier}" + identifier = "#{study.accession}:#{annotation_name}--group--#{annotation_scope}" bucket_files.each do |filepath| remote = ApplicationController.firecloud_client.get_workspace_file(study.bucket_id, filepath) if remote.present? diff --git a/app/models/ingest_job.rb b/app/models/ingest_job.rb index 16f6d1592..c2019b580 100644 --- a/app/models/ingest_job.rb +++ b/app/models/ingest_job.rb @@ -981,7 +981,7 @@ def log_to_mixpanel mixpanel_log_props = get_job_analytics # log job properties to Mixpanel MetricsService.log(mixpanel_event_name, mixpanel_log_props, user) - report_anndata_summary if study_file.is_viz_anndata? && action != :differential_expression + report_anndata_summary if study_file.is_viz_anndata? && action != :ingest_anndata end # set a mixpanel event name based on action @@ -994,7 +994,9 @@ def anndata_summary_props pipelines = ApplicationController.life_sciences_api_client.list_pipelines previous_jobs = pipelines.operations.select do |op| pipeline_args = op.metadata.dig('pipeline', 'actions').first['commands'] - pipeline_args.detect { |c| c == study_file.id.to_s } && !pipeline_args.include?('--differential-expression') + op.done? && + pipeline_args.detect { |c| c == study_file.id.to_s } && + (!pipeline_args.include?('--differential-expression') && !pipeline_args.include?('--subsample')) end # get total runtime from initial extract to final parse initial_extract = previous_jobs.last @@ -1041,18 +1043,30 @@ def anndata_summary_props # report a summary of all AnnData extraction for this file to Mixpanel, if this is the last job def report_anndata_summary + study_file.reload + return false if study_file.has_anndata_summary? # don't bother checking if summary is already sent + + file_identifier = "#{study_file.upload_file_name} (#{study_file.id})" + Rails.logger.info "Checking AnnData summary for #{file_identifier} after #{action}" remaining_jobs = DelayedJobAccessor.find_jobs_by_handler_type(IngestJob, study_file) - fragment = params_object.associated_file # fragment file from this job - # discard the job that corresponds with this ingest process - still_processing = remaining_jobs.reject do |job| + # find running jobs associated with this file that are part of primary extraction (expression, metadata, clustering) + still_processing = remaining_jobs.select do |job| ingest_job = DelayedJobAccessor.dump_job_handler(job).object - ingest_job.params_object.is_a?(AnnDataIngestParameters) && ingest_job.params_object.associated_file == fragment - end.any? + ingest_job.params_object.is_a?(AnnDataIngestParameters) && + !ingest_job.done? && + %i[ingest_cluster ingest_cell_metadata ingest_expression].include?(ingest_job.action) + end - # only continue if there are no more jobs and a summary has not been sent yet - study_file.reload - return false if still_processing || study_file.has_anndata_summary? + if still_processing.any? + files = still_processing.map do |job| + ingest_job = DelayedJobAccessor.dump_job_handler(job).object + "#{ingest_job.action} - #{ingest_job.params_object.associated_file}" + end + Rails.logger.info "Found #{still_processing.count} jobs still processing for #{file_identifier} #{files.join(', ')}" + return false + end + Rails.logger.info "Sending AnnData summary for #{file_identifier} after #{action}" study_file.set_anndata_summary! # prevent race condition leading to duplicate summaries MetricsService.log('ingestSummary', anndata_summary_props, user) end diff --git a/test/models/ingest_job_test.rb b/test/models/ingest_job_test.rb index 4a8d0b4fa..737aa35b3 100644 --- a/test/models/ingest_job_test.rb +++ b/test/models/ingest_job_test.rb @@ -371,6 +371,7 @@ class IngestJobTest < ActiveSupport::TestCase pipeline_mock = MiniTest::Mock.new 4.times {pipeline_mock.expect :metadata, metadata_mock } 2.times {pipeline_mock.expect :error, nil } + pipeline_mock.expect :done?, true operations_mock = Minitest::Mock.new operations_mock.expect :operations, [pipeline_mock] @@ -418,20 +419,27 @@ class IngestJobTest < ActiveSupport::TestCase ingest_cluster: true, name: 'UMAP', cluster_file:, domain_ranges: {}, ingest_anndata: false, extract: nil, obsm_keys: nil ) + pipeline_name = SecureRandom.uuid cluster_job = IngestJob.new( - study: @basic_study, study_file: ann_data_file, user: @user, action: :ingest_cluster, + pipeline_name:, study: @basic_study, study_file: ann_data_file, user: @user, action: :ingest_cluster, params_object: cluster_params_object ) job_mock = Minitest::Mock.new - job_mock.expect :object, cluster_job + 2.times { job_mock.expect :object, cluster_job } + + pipeline_mock = Minitest::Mock.new + pipeline_mock.expect :done?, false # negative test DelayedJobAccessor.stub :find_jobs_by_handler_type, [Delayed::Job.new] do DelayedJobAccessor.stub :dump_job_handler, job_mock do - metadata_job.report_anndata_summary - job_mock.verify - ann_data_file.reload - assert_not ann_data_file.has_anndata_summary? + ApplicationController.life_sciences_api_client.stub :get_pipeline, pipeline_mock do + metadata_job.report_anndata_summary + job_mock.verify + pipeline_mock.verify + ann_data_file.reload + assert_not ann_data_file.has_anndata_summary? + end end end @@ -458,6 +466,7 @@ class IngestJobTest < ActiveSupport::TestCase cluster_job.report_anndata_summary ann_data_file.reload assert ann_data_file.has_anndata_summary? + metrics_mock.verify end end end @@ -475,6 +484,7 @@ class IngestJobTest < ActiveSupport::TestCase failed_pipeline = Minitest::Mock.new 6.times { failed_pipeline.expect(:metadata, failed_metadata) } 3.times { failed_pipeline.expect(:error, true) } + failed_pipeline.expect :done?, true operations_mock = Minitest::Mock.new operations_mock.expect :operations, [failed_pipeline] client_mock = Minitest::Mock.new From f2333f3f452ff82a64dbe23ed4988d6140deb1b4 Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Fri, 27 Sep 2024 14:56:39 -0400 Subject: [PATCH 10/23] Fix "or" prose conversion, refine actionability in error message --- .../lib/validation/validate-file-content.js | 12 ++++++------ test/js/lib/validate-file-content.test.js | 11 +++++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/javascript/lib/validation/validate-file-content.js b/app/javascript/lib/validation/validate-file-content.js index 338ba0fa2..02cf92b98 100644 --- a/app/javascript/lib/validation/validate-file-content.js +++ b/app/javascript/lib/validation/validate-file-content.js @@ -264,7 +264,7 @@ export async function parseClusterFile(chunker, mimeType) { * Example: prettyAndOr(['A', 'B', 'C'], 'or') > '"A", "B", or "C"' */ function prettyAndOr(stringArray, operator) { let phrase - const quoted = stringArray.map(ext => `"${ext}"`) + const quoted = stringArray.map(ext => `".${ext}"`) if (quoted.length === 1) { phrase = quoted[0] @@ -274,7 +274,7 @@ function prettyAndOr(stringArray, operator) { } else if (quoted.length > 2) { // e.g. "A", "B", or "C" const last = quoted.slice(-1)[0] - phrase = `${quoted.slice(-1).join(', ') } ${operator} ${last}` + phrase = `${quoted.slice(0, -1).join(', ')}, ${operator} ${last}` } return phrase @@ -308,10 +308,10 @@ export async function validateGzipEncoding(file, fileType) { } } else { if (firstByte === GZIP_MAGIC_NUMBER) { - const prettyExts = prettyAndOr(EXTENSIONS_MUST_GZIP) - const problem = `Only files with extensions ${prettyExts}) may be gzipped` - const solution = 'please decompress file and retry' - throw new ParseException('encoding:missing-gz-extension', `${problem}; ${solution}`) + const prettyExts = prettyAndOr(EXTENSIONS_MUST_GZIP, 'or') + const problem = `Only files with extensions ${prettyExts} may be gzipped` + const solution = 'Please decompress file, or add a ".gz" extension to the file name, and retry.' + throw new ParseException('encoding:missing-gz-extension', `${problem}. ${solution}`) } else { isGzipped = false } diff --git a/test/js/lib/validate-file-content.test.js b/test/js/lib/validate-file-content.test.js index 0185c972d..ef6555183 100644 --- a/test/js/lib/validate-file-content.test.js +++ b/test/js/lib/validate-file-content.test.js @@ -319,6 +319,17 @@ describe('Client-side file validation', () => { expect(errors[0][1]).toEqual('encoding:missing-gz-extension') }) + it('catches real gzipped file with txt extension', async () => { + const file = createMockFile({ fileName: 'missing_gz_extension.txt'}) + const [{ errors }] = await validateLocalFile(file, { file_type: 'Cluster' }) + expect(errors).toHaveLength(1) + expect(errors[0][1]).toEqual('encoding:missing-gz-extension') + const expectedMessage = + // eslint-disable-next-line max-len + 'Only files with extensions ".gz", ".bam", or ".tbi" may be gzipped. Please decompress file, or add a ".gz" extension to the file name, and retry.' + expect(errors[0][2]).toEqual(expectedMessage) + }) + it('catches text file with .gz suffix', async () => { const file = createMockFile({ fileName: 'foo.gz', content: 'CELL\tX\tY' }) const [{ errors }] = await validateLocalFile(file, { file_type: 'Cluster' }) From e20b541a9b4427989b2ed73b8c7fbb45a6c60134 Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Fri, 27 Sep 2024 15:29:35 -0400 Subject: [PATCH 11/23] Refine error message for better actionability --- .../lib/validation/validate-file-content.js | 2 +- test/js/lib/validate-file-content.test.js | 2 +- .../test_data/validation/missing_gz_extension.txt | Bin 0 -> 277 bytes 3 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 test/test_data/validation/missing_gz_extension.txt diff --git a/app/javascript/lib/validation/validate-file-content.js b/app/javascript/lib/validation/validate-file-content.js index 02cf92b98..2a1c47593 100644 --- a/app/javascript/lib/validation/validate-file-content.js +++ b/app/javascript/lib/validation/validate-file-content.js @@ -310,7 +310,7 @@ export async function validateGzipEncoding(file, fileType) { if (firstByte === GZIP_MAGIC_NUMBER) { const prettyExts = prettyAndOr(EXTENSIONS_MUST_GZIP, 'or') const problem = `Only files with extensions ${prettyExts} may be gzipped` - const solution = 'Please decompress file, or add a ".gz" extension to the file name, and retry.' + const solution = 'Please add a ".gz" extension to the file name, or decompress the file, and retry.' throw new ParseException('encoding:missing-gz-extension', `${problem}. ${solution}`) } else { isGzipped = false diff --git a/test/js/lib/validate-file-content.test.js b/test/js/lib/validate-file-content.test.js index ef6555183..e38ac1f82 100644 --- a/test/js/lib/validate-file-content.test.js +++ b/test/js/lib/validate-file-content.test.js @@ -326,7 +326,7 @@ describe('Client-side file validation', () => { expect(errors[0][1]).toEqual('encoding:missing-gz-extension') const expectedMessage = // eslint-disable-next-line max-len - 'Only files with extensions ".gz", ".bam", or ".tbi" may be gzipped. Please decompress file, or add a ".gz" extension to the file name, and retry.' + 'Only files with extensions ".gz", ".bam", or ".tbi" may be gzipped. Please add a ".gz" extension to the file name, or decompress the file, and retry.' expect(errors[0][2]).toEqual(expectedMessage) }) diff --git a/test/test_data/validation/missing_gz_extension.txt b/test/test_data/validation/missing_gz_extension.txt new file mode 100644 index 0000000000000000000000000000000000000000..c98cae8e00d46f258900afe819985491e67d5889 GIT binary patch literal 277 zcmV+w0qXuAiwFpQz4m4R18r$@b7^j8UuSw>Wq5RDZgXjGZZ33qbO1$AK~BUl47~Rh zqdjk1f#AXw35gSjMO>>ibyK4>iDGA2csQH|OC&*vip`t}<~qdRiX$X~o2sKr zBC9{ft6`7F)6*MFs6b}tC|7YZS=knEGAQSmv6F~#gqa+NP$PDSm*79Imk2&sw|_4= z48V!#H#YT+)!P^ Date: Mon, 30 Sep 2024 12:13:00 -0400 Subject: [PATCH 12/23] hide cell filtering in multi-gene contexts --- .../components/explore/ExploreDisplayPanelManager.jsx | 4 ++-- app/javascript/components/explore/ExploreDisplayTabs.jsx | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx index 9f5b8a0a6..d642702d9 100644 --- a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx +++ b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx @@ -200,7 +200,7 @@ export default function ExploreDisplayPanelManager({ showDifferentialExpressionPanel, setIsCellSelecting, currentPointsSelected, isCellSelecting, deGenes, setDeGenes, setShowDeGroupPicker, cellFaceting, cellFilteringSelection, cellFilterCounts, clusterCanFilter, filterErrorText, - updateFilteredCells, panelToShow, toggleViewOptions + updateFilteredCells, panelToShow, toggleViewOptions, canFilter }) { const [, setRenderForcer] = useState({}) const [dataCache] = useState(createCache()) @@ -213,7 +213,7 @@ export default function ExploreDisplayPanelManager({ hasSpatialGroups = exploreInfo.spatialGroups.length > 0 } - const showCellFiltering = getFeatureFlagsWithDefaults()?.show_cell_facet_filtering && !hasSpatialGroups + const showCellFiltering = getFeatureFlagsWithDefaults()?.show_cell_facet_filtering && !hasSpatialGroups && canFilter // Differential expression settings diff --git a/app/javascript/components/explore/ExploreDisplayTabs.jsx b/app/javascript/components/explore/ExploreDisplayTabs.jsx index 19d9fbc59..99b1882d8 100644 --- a/app/javascript/components/explore/ExploreDisplayTabs.jsx +++ b/app/javascript/components/explore/ExploreDisplayTabs.jsx @@ -235,14 +235,20 @@ export default function ExploreDisplayTabs({ const [showDifferentialExpressionPanel, setShowDifferentialExpressionPanel] = useState(deGenes !== null) const [showUpstreamDifferentialExpressionPanel, setShowUpstreamDifferentialExpressionPanel] = useState(deGenes !== null) + const canFilter = (exploreParams?.genes?.length <= 1 || exploreParams?.consensus !== null) let initialPanel = 'options' if (showDifferentialExpressionPanel || showUpstreamDifferentialExpressionPanel) { initialPanel = 'differential-expression' - } else if (exploreParams.facets !== '') { + } else if (exploreParams.facets !== '' && canFilter) { initialPanel = 'cell-filtering' } const [panelToShow, setPanelToShow] = useState(initialPanel) + // gotcha to hide cell filtering UX if user searches for more than one gene + if (panelToShow == 'cell-filtering' && !canFilter) { + setPanelToShow('options') + } + // Hash of trace label names to the number of points in that trace const [countsByLabelForDe, setCountsByLabelForDe] = useState(null) const showDifferentialExpressionTable = (showViewOptionsControls && deGenes !== null) @@ -696,6 +702,7 @@ export default function ExploreDisplayTabs({ updateFilteredCells={updateFilteredCells} panelToShow={panelToShow} toggleViewOptions={toggleViewOptions} + canFilter={canFilter} /> From e8dd86c0eea844ab2e19e63d366d71f67ad69fd7 Mon Sep 17 00:00:00 2001 From: bistline Date: Mon, 30 Sep 2024 17:16:53 -0400 Subject: [PATCH 13/23] Save filtering state and re-enable when applicable --- .../explore/ExploreDisplayPanelManager.jsx | 6 ++++-- .../components/explore/ExploreDisplayTabs.jsx | 19 +++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx index d642702d9..4dd1c2a96 100644 --- a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx +++ b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx @@ -200,7 +200,7 @@ export default function ExploreDisplayPanelManager({ showDifferentialExpressionPanel, setIsCellSelecting, currentPointsSelected, isCellSelecting, deGenes, setDeGenes, setShowDeGroupPicker, cellFaceting, cellFilteringSelection, cellFilterCounts, clusterCanFilter, filterErrorText, - updateFilteredCells, panelToShow, toggleViewOptions, canFilter + updateFilteredCells, panelToShow, toggleViewOptions, shouldShowFiltering }) { const [, setRenderForcer] = useState({}) const [dataCache] = useState(createCache()) @@ -213,7 +213,9 @@ export default function ExploreDisplayPanelManager({ hasSpatialGroups = exploreInfo.spatialGroups.length > 0 } - const showCellFiltering = getFeatureFlagsWithDefaults()?.show_cell_facet_filtering && !hasSpatialGroups && canFilter + const showCellFiltering = getFeatureFlagsWithDefaults()?.show_cell_facet_filtering && + !hasSpatialGroups && + shouldShowFiltering // Differential expression settings diff --git a/app/javascript/components/explore/ExploreDisplayTabs.jsx b/app/javascript/components/explore/ExploreDisplayTabs.jsx index 99b1882d8..b82d80772 100644 --- a/app/javascript/components/explore/ExploreDisplayTabs.jsx +++ b/app/javascript/components/explore/ExploreDisplayTabs.jsx @@ -235,20 +235,14 @@ export default function ExploreDisplayTabs({ const [showDifferentialExpressionPanel, setShowDifferentialExpressionPanel] = useState(deGenes !== null) const [showUpstreamDifferentialExpressionPanel, setShowUpstreamDifferentialExpressionPanel] = useState(deGenes !== null) - const canFilter = (exploreParams?.genes?.length <= 1 || exploreParams?.consensus !== null) let initialPanel = 'options' if (showDifferentialExpressionPanel || showUpstreamDifferentialExpressionPanel) { initialPanel = 'differential-expression' - } else if (exploreParams.facets !== '' && canFilter) { + } else if (exploreParams.facets !== '') { initialPanel = 'cell-filtering' } const [panelToShow, setPanelToShow] = useState(initialPanel) - // gotcha to hide cell filtering UX if user searches for more than one gene - if (panelToShow == 'cell-filtering' && !canFilter) { - setPanelToShow('options') - } - // Hash of trace label names to the number of points in that trace const [countsByLabelForDe, setCountsByLabelForDe] = useState(null) const showDifferentialExpressionTable = (showViewOptionsControls && deGenes !== null) @@ -260,6 +254,15 @@ export default function ExploreDisplayTabs({ const [cellFilteringSelection, setCellFilteringSelection] = useState(null) + // hide cell filtering in non-applicable settings, but remember state + // will re-enable if user returns to a UX scenario where cell filtering can be re-applied + const shouldShowFiltering = (exploreParams?.genes?.length <= 1 || exploreParams?.consensus !== null) + if (panelToShow === 'cell-filtering' && !shouldShowFiltering) { + togglePanel('options') + } else if (shouldShowFiltering && filteredCells && panelToShow === 'options') { + togglePanel('cell-filtering') + } + // flow/error handling for cell filtering const [clusterCanFilter, setClusterCanFilter] = useState(true) const [filterErrorText, setFilterErrorText] = useState(null) @@ -702,7 +705,7 @@ export default function ExploreDisplayTabs({ updateFilteredCells={updateFilteredCells} panelToShow={panelToShow} toggleViewOptions={toggleViewOptions} - canFilter={canFilter} + shouldShowFiltering={shouldShowFiltering} /> From 2bf746654bfc4a9c5c437333ef62dab4e97cae3e Mon Sep 17 00:00:00 2001 From: bistline Date: Tue, 1 Oct 2024 12:34:18 -0400 Subject: [PATCH 14/23] Keep filtering button with proper messaging, adding tests --- .../explore/ExploreDisplayPanelManager.jsx | 28 +++++--- .../components/explore/ExploreDisplayTabs.jsx | 23 ++++--- .../components/explore/PlotTabs.jsx | 5 +- test/js/explore/explore-display-tabs.test.js | 66 +++++++++++++++++++ 4 files changed, 103 insertions(+), 19 deletions(-) diff --git a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx index 4dd1c2a96..868be4783 100644 --- a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx +++ b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx @@ -200,7 +200,7 @@ export default function ExploreDisplayPanelManager({ showDifferentialExpressionPanel, setIsCellSelecting, currentPointsSelected, isCellSelecting, deGenes, setDeGenes, setShowDeGroupPicker, cellFaceting, cellFilteringSelection, cellFilterCounts, clusterCanFilter, filterErrorText, - updateFilteredCells, panelToShow, toggleViewOptions, shouldShowFiltering + updateFilteredCells, panelToShow, toggleViewOptions, allowCellFiltering }) { const [, setRenderForcer] = useState({}) const [dataCache] = useState(createCache()) @@ -213,9 +213,7 @@ export default function ExploreDisplayPanelManager({ hasSpatialGroups = exploreInfo.spatialGroups.length > 0 } - const showCellFiltering = getFeatureFlagsWithDefaults()?.show_cell_facet_filtering && - !hasSpatialGroups && - shouldShowFiltering + const showCellFiltering = getFeatureFlagsWithDefaults()?.show_cell_facet_filtering && !hasSpatialGroups // Differential expression settings @@ -361,7 +359,7 @@ export default function ExploreDisplayPanelManager({ } - {showCellFiltering && panelToShow === 'cell-filtering' && + {showCellFiltering && panelToShow === 'cell-filtering' && allowCellFiltering && + deGenes={deGenes}/> } {panelToShow === 'differential-expression' && } - { showCellFiltering && clusterCanFilter && + { showCellFiltering && clusterCanFilter && allowCellFiltering && <>
@@ -479,6 +476,21 @@ export default function ExploreDisplayPanelManager({
} + { showCellFiltering && !allowCellFiltering && + <> +
+
+ +
+
+ + } { @@ -705,7 +708,7 @@ export default function ExploreDisplayTabs({ updateFilteredCells={updateFilteredCells} panelToShow={panelToShow} toggleViewOptions={toggleViewOptions} - shouldShowFiltering={shouldShowFiltering} + allowCellFiltering={allowCellFiltering} />
diff --git a/app/javascript/components/explore/PlotTabs.jsx b/app/javascript/components/explore/PlotTabs.jsx index 7fd458d1b..5d4d0c280 100644 --- a/app/javascript/components/explore/PlotTabs.jsx +++ b/app/javascript/components/explore/PlotTabs.jsx @@ -55,7 +55,10 @@ export default function PlotTabs({ const tooltip = disabledTooltips[tabKey] const numGenes = tooltip.numToSearch const geneText = `gene${tooltip.isMulti ? 's' : ''}` - const text = `To show this plot, search ${numGenes} ${geneText} using the box at left` + let text = `To show this plot, search ${numGenes} ${geneText} using the box at left` + if (['scatter', 'distribution'].includes(tabKey)) { + text += " or use the 'Collapse genes by' menu for multiple genes" + } return (
  • { } }) +// Prevent "ReferenceError: $ is not defined" errors when trying to call $(window) functions +jest.mock('lib/plot', () => { + return { + getPlotDimensions: jest.fn(() => { return { width: 100, height: 100 } }) + } +}) + import React from 'react' import { render, screen, waitFor } from '@testing-library/react' import * as UserProvider from '~/providers/UserProvider' @@ -424,6 +431,64 @@ describe('explore tabs are activated based on study info and parameters', () => expect(screen.getByTestId('cell-filtering-button')).toHaveTextContent('Filter plotted cells') }) + it('hides cell filtering when not applicable', async () => { + jest + .spyOn(UserProvider, 'getFeatureFlagsWithDefaults') + .mockReturnValue({ + show_cell_facet_filtering: true + }) + + const newExplore = { + 'cluster': 'All Cells UMAP', + 'annotation': { + 'name': 'biosample_id', + 'type': 'group', + 'scope': 'study' + }, + 'consensus': null, + 'genes': ['A1BG', 'A1BG-AS1'] + } + render(( + + )) + expect(screen.getByTestId('cell-filtering-button-disabled')).toHaveTextContent('Filtering unavailable') + }) + + it('shows cell filtering when using consensus metric', async () => { + jest + .spyOn(UserProvider, 'getFeatureFlagsWithDefaults') + .mockReturnValue({ + show_cell_facet_filtering: true + }) + + const newExplore = { + 'cluster': 'All Cells UMAP', + 'annotation': { + 'name': 'biosample_id', + 'type': 'group', + 'scope': 'study' + }, + 'consensus': 'median', + 'genes': ['A1BG', 'A1BG-AS1'], + 'tab': 'scatter', + 'facets': '' + } + render(( + + )) + expect(screen.getByTestId('cell-filtering-button')).toHaveTextContent('Filter plotted cells') + }) + it('disables cell filtering button', async () => { jest .spyOn(UserProvider, 'getFeatureFlagsWithDefaults') @@ -440,6 +505,7 @@ describe('explore tabs are activated based on study info and parameters', () => clusterCanFilter={false} filterErrorText={'Cluster is not indexed'} panelToShow={'options'} + allowCellFiltering={true} /> ) From 2dceba4d7ab63a2f7999586dacc584e4c51c7b26 Mon Sep 17 00:00:00 2001 From: bistline Date: Tue, 1 Oct 2024 13:50:00 -0400 Subject: [PATCH 15/23] Removing erroneous tooltip re: subsampling --- .../components/explore/ExploreDisplayPanelManager.jsx | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx index 868be4783..bfb1969cb 100644 --- a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx +++ b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx @@ -244,14 +244,7 @@ export default function ExploreDisplayPanelManager({ const shownAnnotation = getShownAnnotation(exploreParamsWithDefaults.annotation, annotationList) - const isSubsampled = exploreParamsWithDefaults.subsample !== 'all' let cellFilteringTooltipAttrs = {} - if (isSubsampled) { - cellFilteringTooltipAttrs = { - 'data-toggle': 'tooltip', - 'data-original-title': 'Clicking will remove subsampling; plots might be noticeably slower.' - } - } /** Toggle cell filtering panel */ function toggleCellFilterPanel() { From 7e9ff2c53de55e844f2f3d4d0d6ab8aaffc7cffc Mon Sep 17 00:00:00 2001 From: bistline Date: Wed, 2 Oct 2024 09:45:06 -0400 Subject: [PATCH 16/23] Prevent duplicate button in error state --- .../components/explore/ExploreDisplayPanelManager.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx index bfb1969cb..c9ccc9a3c 100644 --- a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx +++ b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx @@ -454,7 +454,7 @@ export default function ExploreDisplayPanelManager({ } - { showCellFiltering && !clusterCanFilter && + { showCellFiltering && !clusterCanFilter && allowCellFiltering && <>
    From 4f6f38700b399afb592ee70c255f3a1c9ddc908e Mon Sep 17 00:00:00 2001 From: bistline Date: Wed, 2 Oct 2024 11:57:59 -0400 Subject: [PATCH 17/23] Ignore unsynced directories in bulk download --- app/controllers/api/v1/bulk_download_controller.rb | 2 +- test/api/bulk_download_controller_test.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v1/bulk_download_controller.rb b/app/controllers/api/v1/bulk_download_controller.rb index cbbcc2ef1..80da4ee9a 100644 --- a/app/controllers/api/v1/bulk_download_controller.rb +++ b/app/controllers/api/v1/bulk_download_controller.rb @@ -461,7 +461,7 @@ def self.find_matching_directories(directory_name, file_type, accession) when 'nodirs' [] when 'all' - study.directory_listings.all + study&.directory_listings&.are_synced || [] else DirectoryListing.where(name: sanitized_dirname, study_id: study.id, sync_status: true, file_type: file_type) end diff --git a/test/api/bulk_download_controller_test.rb b/test/api/bulk_download_controller_test.rb index 2d8b7bca7..689f5eaa7 100644 --- a/test/api/bulk_download_controller_test.rb +++ b/test/api/bulk_download_controller_test.rb @@ -484,4 +484,18 @@ class BulkDownloadControllerTest < ActionDispatch::IntegrationTest assert_equal %w[FakeHCAProject AnotherFakeHCAProject].sort, hca_accessions.sort end end + + test 'should ignore unsynced directories' do + study = FactoryBot.create(:detached_study, name: 'Unsynced Dir Test', user: @user, test_array: @@studies_to_clean) + files = 1.upto(30).map do |i| + { + name: "_scp_internal/subdir/output_#{i}.tsv", + size: i * 100, + generation: SecureRandom.random_number(10000..99999) + } + end + directory = DirectoryListing.create(study:, file_type: 'tsv', name: '_scp_internal', files:, sync_status: false) + assert directory.persisted? + assert_empty Api::V1::BulkDownloadController.find_matching_directories('all', '', study.accession) + end end From ca12d1d9df4c87d18680dbabb4b143d4d09a0002 Mon Sep 17 00:00:00 2001 From: bistline Date: Wed, 2 Oct 2024 16:24:15 -0400 Subject: [PATCH 18/23] Don't create default cluster fragment for reference AnnData files --- app/models/ann_data_file_info.rb | 2 +- app/models/ingest_job.rb | 2 +- test/models/ann_data_file_info_test.rb | 7 ++++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/models/ann_data_file_info.rb b/app/models/ann_data_file_info.rb index d0d9184ef..51a935ed9 100644 --- a/app/models/ann_data_file_info.rb +++ b/app/models/ann_data_file_info.rb @@ -193,7 +193,7 @@ def sanitize_fragments! # create the default cluster data_fragment entries def set_default_cluster_fragments! - return false if fragments_by_type(:cluster).any? + return false if fragments_by_type(:cluster).any? || reference_file default_obsm_keys = AnnDataIngestParameters::PARAM_DEFAULTS[:obsm_keys] default_obsm_keys.each do |obsm_key_name| diff --git a/app/models/ingest_job.rb b/app/models/ingest_job.rb index 16f6d1592..6582385bb 100644 --- a/app/models/ingest_job.rb +++ b/app/models/ingest_job.rb @@ -347,7 +347,7 @@ def poll_for_completion(run_at: 1.minute.from_now) if done? && !failed? Rails.logger.info "IngestJob poller: #{pipeline_name} is done!" Rails.logger.info "IngestJob poller: #{pipeline_name} status: #{current_status}" - unless special_action? || action == :ingest_anndata + unless special_action? || (action == :ingest_anndata && study_file.is_viz_anndata?) study_file.update(parse_status: 'parsed') study_file.bundled_files.each { |sf| sf.update(parse_status: 'parsed') } end diff --git a/test/models/ann_data_file_info_test.rb b/test/models/ann_data_file_info_test.rb index fd7a80167..dfbc17228 100644 --- a/test/models/ann_data_file_info_test.rb +++ b/test/models/ann_data_file_info_test.rb @@ -127,7 +127,7 @@ def generate_id end test 'should set default cluster fragments' do - ann_data_info = AnnDataFileInfo.new + ann_data_info = AnnDataFileInfo.new(reference_file: false) assert ann_data_info.valid? default_keys = AnnDataIngestParameters::PARAM_DEFAULTS[:obsm_keys] default_keys.each do |obsm_key_name| @@ -135,6 +135,11 @@ def generate_id matcher = { data_type: :cluster, name:, obsm_key_name: }.with_indifferent_access assert ann_data_info.find_fragment(**matcher).present? end + # ensure non-parseable AnnData files don't create fragment + reference_anndata = AnnDataFileInfo.new + assert reference_anndata.valid? + assert_empty reference_anndata.data_fragments + assert_empty reference_anndata.obsm_key_names end test 'should validate data fragments' do From 72544ffdb774ef6f623567c15b2acd79be7eff70 Mon Sep 17 00:00:00 2001 From: bistline Date: Thu, 3 Oct 2024 09:58:39 -0400 Subject: [PATCH 19/23] addressing PR comments re: maintainability --- app/models/ingest_job.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/models/ingest_job.rb b/app/models/ingest_job.rb index c2019b580..ec3f8b969 100644 --- a/app/models/ingest_job.rb +++ b/app/models/ingest_job.rb @@ -32,6 +32,9 @@ class IngestJob # steps that need to be accounted for SPECIAL_ACTIONS = %i[differential_expression render_expression_arrays image_pipeline].freeze + # main processes that extract or ingest data for core visualizations (scatter, violin, dot, etc) + CORE_ACTIONS = %w[ingest_anndata ingest_expression ingest_cell_metadata ingest_cluster] + # jobs that need parameters objects in order to launch correctly PARAMS_OBJ_REQUIRED = %i[ differential_expression render_expression_arrays image_pipeline ingest_anndata @@ -996,7 +999,7 @@ def anndata_summary_props pipeline_args = op.metadata.dig('pipeline', 'actions').first['commands'] op.done? && pipeline_args.detect { |c| c == study_file.id.to_s } && - (!pipeline_args.include?('--differential-expression') && !pipeline_args.include?('--subsample')) + (pipeline_args & CORE_ACTIONS).any? end # get total runtime from initial extract to final parse initial_extract = previous_jobs.last From 5597eabaf9fb3e7dc9eaaaf2cfe706c9ae2f298e Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Thu, 3 Oct 2024 10:00:05 -0400 Subject: [PATCH 20/23] Omit alphanumeric-underscore validation for cluster files --- .../lib/validation/shared-validation.js | 20 +++++++++++++++++-- .../lib/validation/validate-file-content.js | 8 ++++---- ... cluster_valid_annotation_name_period.txt} | 0 3 files changed, 22 insertions(+), 6 deletions(-) rename test/test_data/{cluster_invalid_annotation_name_period.txt => cluster_valid_annotation_name_period.txt} (100%) diff --git a/app/javascript/lib/validation/shared-validation.js b/app/javascript/lib/validation/shared-validation.js index 9361e3c7f..8f902120d 100644 --- a/app/javascript/lib/validation/shared-validation.js +++ b/app/javascript/lib/validation/shared-validation.js @@ -232,12 +232,28 @@ export function validateUnique(headers) { } /** - * Check headers for disallowed characters + * Check headers for disallowed characters in metadata annotations + * + * This rule exists because BigQuery does not except e.g. periods in its + * column names, without special quoting. We use BigQuery to enable cross-study + * search on annotations like "species", "disease", etc. + * + * Cluster-specific annotations aren't searchable in cross-study search, so + * we skip this rule for cluster files (via `false` for `hasMetadataAnnotations`). + * + * More context: https://github.com/broadinstitute/single_cell_portal_core/pull/2143 */ -export function validateAlphanumericAndUnderscores(headers) { +export function validateAlphanumericAndUnderscores(headers, hasMetadataAnnotations=true) { // eslint-disable-next-line max-len // Mirrors https://github.com/broadinstitute/scp-ingest-pipeline/blob/7c3ea039683c3df90d6e32f23bf5e6813d8fbaba/ingest/validation/validate_metadata.py#L1223 const issues = [] + + if (!hasMetadataAnnotations) { + // Skip this validation for cluster files + return issues + } + + const uniques = new Set(headers) const prohibitedChars = new RegExp(/[^A-Za-z0-9_]/) diff --git a/app/javascript/lib/validation/validate-file-content.js b/app/javascript/lib/validation/validate-file-content.js index 740a3464e..63bdccb2e 100644 --- a/app/javascript/lib/validation/validate-file-content.js +++ b/app/javascript/lib/validation/validate-file-content.js @@ -154,16 +154,16 @@ function validateEqualCount(headers, annotTypes) { * Cap lines are like meta-information lines in other file formats * (e.g. VCF), but do not begin with pound signs (#). */ -function validateCapFormat([headers, annotTypes]) { +function validateCapFormat([headers, annotTypes], isMetadataFile=true) { let issues = [] if (!headers || !annotTypes) { return [['error', 'format:cap:no-cap-rows', 'File does not have 2 non-empty header rows']] } - // Check format rules that apply to both metadata and cluster files + // Check format rules that apply to both metadata and (except one rule) cluster files issues = issues.concat( validateUnique(headers), - validateAlphanumericAndUnderscores(headers), + validateAlphanumericAndUnderscores(headers, isMetadataFile), validateNameKeyword(headers), validateTypeKeyword(annotTypes), validateGroupOrNumeric(annotTypes), @@ -241,7 +241,7 @@ export async function parseMetadataFile(chunker, mimeType, fileOptions) { /** parse a cluster file, and return an array of issues, along with file parsing info */ export async function parseClusterFile(chunker, mimeType) { const { headers, delimiter } = await getParsedHeaderLines(chunker, mimeType) - let issues = validateCapFormat(headers) + let issues = validateCapFormat(headers, false) issues = issues.concat(validateClusterCoordinates(headers)) // add other header validations here diff --git a/test/test_data/cluster_invalid_annotation_name_period.txt b/test/test_data/cluster_valid_annotation_name_period.txt similarity index 100% rename from test/test_data/cluster_invalid_annotation_name_period.txt rename to test/test_data/cluster_valid_annotation_name_period.txt From 631f41598fd7ab02f8aa69613cef9221a1ed285f Mon Sep 17 00:00:00 2001 From: bistline Date: Thu, 3 Oct 2024 11:14:04 -0400 Subject: [PATCH 21/23] Account for raw_counts in num_files_extracted --- app/models/ingest_job.rb | 12 ++++++++++++ test/models/ingest_job_test.rb | 6 +++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/models/ingest_job.rb b/app/models/ingest_job.rb index ec3f8b969..05c5fb56d 100644 --- a/app/models/ingest_job.rb +++ b/app/models/ingest_job.rb @@ -1028,6 +1028,7 @@ def anndata_summary_props op.metadata.dig('pipeline', 'actions').first['commands'].detect { |c| c == '--extract' } || op.error.present? end.count + num_files_extracted += 1 if extracted_raw_counts?(initial_extract.metadata) # event properties for Mixpanel summary event { perfTime: job_perftime, @@ -1044,6 +1045,17 @@ def anndata_summary_props } end + # determine if an ingest_anndata job extracted raw counts data + # reads from the --extract parameter to avoid counting filenames that include 'raw_counts' + def extracted_raw_counts?(pipeline_metadata) + commands = pipeline_metadata.dig('pipeline', 'actions').first['commands'] + extract_idx = commands.index('--extract') + return false if extract_idx.nil? + + extract_params = commands[extract_idx + 1] + extract_params.include?('raw_counts') + end + # report a summary of all AnnData extraction for this file to Mixpanel, if this is the last job def report_anndata_summary study_file.reload diff --git a/test/models/ingest_job_test.rb b/test/models/ingest_job_test.rb index 737aa35b3..2bae7f2ae 100644 --- a/test/models/ingest_job_test.rb +++ b/test/models/ingest_job_test.rb @@ -333,7 +333,7 @@ class IngestJobTest < ActiveSupport::TestCase end end - test 'should get ingest summary for AnnData parsing' do + test 'should get ingestSummary for AnnData parsing' do ann_data_file = FactoryBot.create(:ann_data_file, name: 'data.h5ad', study: @basic_study) ann_data_file.ann_data_file_info.reference_file = false ann_data_file.ann_data_file_info.data_fragments = [ @@ -369,7 +369,7 @@ class IngestJobTest < ActiveSupport::TestCase }.with_indifferent_access pipeline_mock = MiniTest::Mock.new - 4.times {pipeline_mock.expect :metadata, metadata_mock } + 5.times {pipeline_mock.expect :metadata, metadata_mock } 2.times {pipeline_mock.expect :error, nil } pipeline_mock.expect :done?, true @@ -482,7 +482,7 @@ class IngestJobTest < ActiveSupport::TestCase pipeline = { actions: } failed_metadata = { pipeline:, events:, startTime: (now - 1.hour).to_s, endTime: now.to_s }.with_indifferent_access failed_pipeline = Minitest::Mock.new - 6.times { failed_pipeline.expect(:metadata, failed_metadata) } + 7.times { failed_pipeline.expect(:metadata, failed_metadata) } 3.times { failed_pipeline.expect(:error, true) } failed_pipeline.expect :done?, true operations_mock = Minitest::Mock.new From dfd7c3142f4a7d57891ac6d93d322e06b0a286a1 Mon Sep 17 00:00:00 2001 From: Eric Weitz Date: Thu, 3 Oct 2024 12:14:38 -0400 Subject: [PATCH 22/23] Refine test for disallowed characters in headers --- test/js/lib/validate-file-content.test.js | 18 ++++++++++++------ .../cluster_valid_annotation_name_period.txt | 0 ...metadata_invalid_annotation_name_period.tsv | 0 3 files changed, 12 insertions(+), 6 deletions(-) rename test/test_data/{ => validation}/cluster_valid_annotation_name_period.txt (100%) rename test/test_data/{ => validation}/metadata_invalid_annotation_name_period.tsv (100%) diff --git a/test/js/lib/validate-file-content.test.js b/test/js/lib/validate-file-content.test.js index 36fa32fbf..7eed5800a 100644 --- a/test/js/lib/validate-file-content.test.js +++ b/test/js/lib/validate-file-content.test.js @@ -421,20 +421,26 @@ describe('Client-side file validation', () => { expect(displayedWarning).toHaveTextContent(issues.warnings[0][2]) }) - it('Catches disallowed characters in header', async () => { + it('Does not throw disallowed characters in cluster header', async () => { const file = createMockFile({ fileName: 'foo.txt', content: 'NAME,X,Y,invalid.header\nTYPE,numeric,numeric,numeric,numeric\nCELL_0001,34.472,32.211\nCELL_0002,15.975,10.043,5' }) const [{ errors }] = await validateLocalFile(file, { file_type: 'Cluster' }) - expect(errors).toHaveLength(1) - - const expectedErrorType = 'format:cap:only-alphanumeric-underscore' - const errorType = errors[0][1] - expect(errorType).toBe(expectedErrorType) + expect(errors).toHaveLength(0) }) }) +it('Catches disallowed characters in metadata header', async () => { + const file = createMockFile({ fileName: 'metadata_invalid_annotation_name_period.tsv' }) + const [{ errors }] = await validateLocalFile(file, { file_type: 'Metadata' }) + expect(errors).toHaveLength(1) + + const expectedErrorType = 'format:cap:only-alphanumeric-underscore' + const errorType = errors[0][1] + expect(errorType).toBe(expectedErrorType) +}) + // With the client side file validation feature flag set to false expect invalid files to pass describe('Client-side file validation feature flag is false', () => { beforeEach(() => { diff --git a/test/test_data/cluster_valid_annotation_name_period.txt b/test/test_data/validation/cluster_valid_annotation_name_period.txt similarity index 100% rename from test/test_data/cluster_valid_annotation_name_period.txt rename to test/test_data/validation/cluster_valid_annotation_name_period.txt diff --git a/test/test_data/metadata_invalid_annotation_name_period.tsv b/test/test_data/validation/metadata_invalid_annotation_name_period.tsv similarity index 100% rename from test/test_data/metadata_invalid_annotation_name_period.tsv rename to test/test_data/validation/metadata_invalid_annotation_name_period.tsv From c73bcb266e94215771bc76c7812797ab3a6a9e33 Mon Sep 17 00:00:00 2001 From: bistline Date: Tue, 8 Oct 2024 12:20:43 -0400 Subject: [PATCH 23/23] Update filtering tooltip to indicate data is unfiltered --- .../components/explore/ExploreDisplayPanelManager.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx index c9ccc9a3c..d4d80aed6 100644 --- a/app/javascript/components/explore/ExploreDisplayPanelManager.jsx +++ b/app/javascript/components/explore/ExploreDisplayPanelManager.jsx @@ -478,7 +478,7 @@ export default function ExploreDisplayPanelManager({ className={`btn btn-primary`} data-testid="cell-filtering-button-disabled" data-toggle="tooltip" - data-original-title={`Cell filtering cannot be shown in this context (only scatter or distribution tabs)`} + data-original-title={`Cell filtering cannot be applied in this context (only scatter or distribution tabs) - visible plots are unfiltered`} >Filtering unavailable