📁 Feat/55 Load Files by demariadaniel · Pull Request #210 · overture-stack/stage

demariadaniel · 2024-09-10T17:40:16Z

Loads selected file from Table Data in Iobio visualizer
Requests object URL from Score on table page load
Basic error handling for 0 files, multiple files, and wrong file types
Demo File URL for testing visualizer integration while demo files in Arranger are updated

Screen.Recording.2024-09-17.at.3.13.07.PM.mov

…feat/55-load-files

ciaranschutte · 2024-09-24T14:16:50Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


is this needed if there's this https://github.com/overture-stack/stage/pull/210/files#diff-d22d109917820bd9b112220302d1f28113412fb3db7c40ada3d03d835f5aacc1R30 typeguard in use?

It's because of tableData.find, which returns : unknown
tableData is unknown[] || []

This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

Updated in 33629a4
As discussed on Slack the .filter as would ideally not be needed but something about the type guard rowIsFileData isn't working quite as intended.

ciaranschutte · 2024-09-24T14:21:49Z

+export const baseScoreDownloadParams = {
+	external: 'true',
+	offset: '0',
+	'User-Agent': 'unknown',
+};


interesting note - these can be their actual types and not only strings because using URLSearchParams will format them correctly as needed

Good idea but URLSearchParams is actually expecting Record<string, string>

Where is the type coming from?
I checked this works:
newURLSearchParams({ext: true,count: 8,x: 'a'}).toString()
also check 2nd example here:
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#examples

Errors when updating types/values as described

Checked this again and seeing these same errors using the example you provided so this is a good one to compare notes on

ciaranschutte · 2024-09-24T23:21:29Z

 export const getScoreDownloadUrls = async (fileData: FileTableData) => {
 	const { NEXT_PUBLIC_SCORE_API_URL } = getConfig();
-	const length = fileData.file.size.toString();
+	const length = fileData.file?.size?.toString();


This is optional chaining
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
not nullish coalescing
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing
and will potentially return undefined which does not have the toString method

mentioning nullish coalescing because I think they're being confused in this PR 803ddab

I used the wrong term but optional chaining does protect against errors in this case. Tested in Node.
const test = null; test?.key?.data?.toString(); // returns undefined test.toString(); // throws error
I'll double check the logic. I added this to handle issues encountered with new demo data added yesterday. It was working at the time of testing.

ciaranschutte

few bits of React improvement:

regarding asyncs and this comment
#210 (comment)
I think you might need to follow the example down to this section in the link you shared: https://devtrium.com/posts/async-functions-useeffect#what-if-you-need-to-extract-the-function-outside-useeffect

smaller ones:

nested components not needed
no need for useMemo
static vars inside components

looks like the core bit of state we care about is the fileUrl so my gut is telling me that there's improvements we could make in computed that but I don't have bandwidth for that right now. I imagine it's along the lines of just passing in a fileUrl and loading props to the BamTable and leaving the reasoning about files somewhere else.

ciaranschutte · 2024-09-25T02:08:29Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

ciaranschutte · 2024-09-25T02:12:47Z

+	const response = await axios.get(
+ urlJoin(NEXT_PUBLIC_SCORE_API_URL, SCORE_API_DOWNLOAD_PATH, object_id, `?${urlParams}`),
+ {
+ headers: { accept: '*/*' },
+ },
+	);
+
+	if (response.status === 200) {
+ return response.data;
+	}
+	console.error(`Error at getScoreDownloadUrls with object_id ${object_id}`);
+	throw new Error(`Error at getScoreDownloadUrls status: ${response.status}, ok: false`);
+};


Does this work with errors?
I think there's an interceptor in components/utils that will Promise.reject(err)
but I don't think response will necessarily have a status field in this case

OK - axios was throwing an error, but it was printing to console, and not reading these lines.
I've straightened this out with Axios async/try/catch. Errors yield infinite loading state.

ciaranschutte · 2024-09-25T02:27:38Z

 );

-const BamTable = () => {
+const BamTable = ({ file }: { file: FileTableData | undefined }) => {


What happens if undefined is passed in as a prop?
I can get a local setup working tomorrow if I have time but seems like it might stay stuck on loading state and console logs an error - which isn't a working component

You are correct that is what happens. Right now this is managed by disabling the files/visualization button when the bamFile is undefined. There is no URL management for bamFile selected or FileTable/BamTable. File is undefined on page load.

ciaranschutte · 2024-09-25T02:30:05Z

+	const DemoDataButton = () => (
+ <div>
+ <button
+ css={css`
+ border: 2px solid ${theme.colors.accent};
+ border-radius: 5px;
+ min-width: fit-content;
+ padding: 3px 10px;
+ ${getToggleButtonStyles(isDemoData, theme)}
+ `}
+ onClick={loadDemoFile}
+ >
+ {isDemoData ? 'View File Data' : 'View Demo Data'}
+ </button>
+ </div>
+	);


I don't think we should necessarily be creating "demo" specific code.
But if we have to this is a component and should exist by itself and take props.
This isolates it significantly more from what is our production? real? code plus it's just correct to not have this be created on every render

I will revisit React / code quality comments. To clarify:
This is a feature branch deployed to its own URL on a project that is deprioritized for the next 3-6 months due to project scheduling requirements.
Demo specific code is provided due to the fact that the files currently hosted in Song/Arranger are not working with the Iobio server. This is a data compatibility issue that needs additional troubleshooting.
The Demo code is there to facilitate short term UI testing while the data issues are resolved. Without an option to load the demo data the Visualization page displays multiple NaN and blank graphs. So without the Demo data there's no testing the Iobio 'happy path', which does work.
I've been asked to move off this project temporarily so without time to devote to solving this properly I've added a temporary solution to enable other team members to continue working.

moved: #210 (comment)

ciaranschutte · 2024-09-25T03:06:43Z

+	/* TODO: Remove Demo Data logic */
+	const demoFileMetadata: FileMetaData = {
+ objectId: 'demoFileData',
+ parts: [
+ {
+ url: 'https://s3.amazonaws.com/iobio/NA12878/NA12878.autsome.bam',
+ },
+ ],
+	};


move out of component

Moved: Move Demo Data to its own file
I was trying to centralize all Demo Data logic so that removal wouldn't require scouring several places, see discussion here:
#210 (comment)
Because the Demo Data logic relies on hooks and state in the parent component I had it all grouped in one block with the understanding it was only temporary.
I've moved the Component and metadata constant so they are not constantly redeclared but the remaining logic is still tied to the parent component.

I'd ask what about demoFileMetadata is tied to a hook or state? It looks like a regular object with static property values.

Excellent - moved everything into the Demo Button component. Just needed a day to sleep on it.
There's some logic to manage switching from the Demo file back to a real file that needed untangling but it should be figured out now.

demariadaniel · 2024-09-25T15:05:44Z

Missing await added: Add missing await

@ciaranschutte re: #210 (comment)

ciaranschutte

offline discussion of context of this feature branch - we are good to approve as is and improve upon later

demariadaniel · 2024-09-10T17:40:16Z

Loads selected file from Table Data in Iobio visualizer
Requests object URL from Score on table page load
Basic error handling for 0 files, multiple files, and wrong file types
Demo File URL for testing visualizer integration while demo files in Arranger are updated

Screen.Recording.2024-09-17.at.3.13.07.PM.mov

…feat/55-load-files

ciaranschutte · 2024-09-24T14:16:50Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


is this needed if there's this https://github.com/overture-stack/stage/pull/210/files#diff-d22d109917820bd9b112220302d1f28113412fb3db7c40ada3d03d835f5aacc1R30 typeguard in use?

It's because of tableData.find, which returns : unknown
tableData is unknown[] || []

This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

Updated in 33629a4
As discussed on Slack the .filter as would ideally not be needed but something about the type guard rowIsFileData isn't working quite as intended.

ciaranschutte · 2024-09-24T14:21:49Z

+export const baseScoreDownloadParams = {
+	external: 'true',
+	offset: '0',
+	'User-Agent': 'unknown',
+};


interesting note - these can be their actual types and not only strings because using URLSearchParams will format them correctly as needed

Good idea but URLSearchParams is actually expecting Record<string, string>

Where is the type coming from?
I checked this works:
newURLSearchParams({ext: true,count: 8,x: 'a'}).toString()
also check 2nd example here:
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#examples

Errors when updating types/values as described

Checked this again and seeing these same errors using the example you provided so this is a good one to compare notes on

ciaranschutte · 2024-09-24T23:21:29Z

 export const getScoreDownloadUrls = async (fileData: FileTableData) => {
 	const { NEXT_PUBLIC_SCORE_API_URL } = getConfig();
-	const length = fileData.file.size.toString();
+	const length = fileData.file?.size?.toString();


This is optional chaining
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
not nullish coalescing
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing
and will potentially return undefined which does not have the toString method

mentioning nullish coalescing because I think they're being confused in this PR 803ddab

I used the wrong term but optional chaining does protect against errors in this case. Tested in Node.
const test = null; test?.key?.data?.toString(); // returns undefined test.toString(); // throws error
I'll double check the logic. I added this to handle issues encountered with new demo data added yesterday. It was working at the time of testing.

ciaranschutte

few bits of React improvement:

regarding asyncs and this comment
#210 (comment)
I think you might need to follow the example down to this section in the link you shared: https://devtrium.com/posts/async-functions-useeffect#what-if-you-need-to-extract-the-function-outside-useeffect

smaller ones:

nested components not needed
no need for useMemo
static vars inside components

looks like the core bit of state we care about is the fileUrl so my gut is telling me that there's improvements we could make in computed that but I don't have bandwidth for that right now. I imagine it's along the lines of just passing in a fileUrl and loading props to the BamTable and leaving the reasoning about files somewhere else.

ciaranschutte · 2024-09-25T02:08:29Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

ciaranschutte · 2024-09-25T02:12:47Z

+	const response = await axios.get(
+ urlJoin(NEXT_PUBLIC_SCORE_API_URL, SCORE_API_DOWNLOAD_PATH, object_id, `?${urlParams}`),
+ {
+ headers: { accept: '*/*' },
+ },
+	);
+
+	if (response.status === 200) {
+ return response.data;
+	}
+	console.error(`Error at getScoreDownloadUrls with object_id ${object_id}`);
+	throw new Error(`Error at getScoreDownloadUrls status: ${response.status}, ok: false`);
+};


Does this work with errors?
I think there's an interceptor in components/utils that will Promise.reject(err)
but I don't think response will necessarily have a status field in this case

OK - axios was throwing an error, but it was printing to console, and not reading these lines.
I've straightened this out with Axios async/try/catch. Errors yield infinite loading state.

ciaranschutte · 2024-09-25T02:27:38Z

 );

-const BamTable = () => {
+const BamTable = ({ file }: { file: FileTableData | undefined }) => {


What happens if undefined is passed in as a prop?
I can get a local setup working tomorrow if I have time but seems like it might stay stuck on loading state and console logs an error - which isn't a working component

You are correct that is what happens. Right now this is managed by disabling the files/visualization button when the bamFile is undefined. There is no URL management for bamFile selected or FileTable/BamTable. File is undefined on page load.

ciaranschutte · 2024-09-25T02:30:05Z

+	const DemoDataButton = () => (
+ <div>
+ <button
+ css={css`
+ border: 2px solid ${theme.colors.accent};
+ border-radius: 5px;
+ min-width: fit-content;
+ padding: 3px 10px;
+ ${getToggleButtonStyles(isDemoData, theme)}
+ `}
+ onClick={loadDemoFile}
+ >
+ {isDemoData ? 'View File Data' : 'View Demo Data'}
+ </button>
+ </div>
+	);


I don't think we should necessarily be creating "demo" specific code.
But if we have to this is a component and should exist by itself and take props.
This isolates it significantly more from what is our production? real? code plus it's just correct to not have this be created on every render

I will revisit React / code quality comments. To clarify:
This is a feature branch deployed to its own URL on a project that is deprioritized for the next 3-6 months due to project scheduling requirements.
Demo specific code is provided due to the fact that the files currently hosted in Song/Arranger are not working with the Iobio server. This is a data compatibility issue that needs additional troubleshooting.
The Demo code is there to facilitate short term UI testing while the data issues are resolved. Without an option to load the demo data the Visualization page displays multiple NaN and blank graphs. So without the Demo data there's no testing the Iobio 'happy path', which does work.
I've been asked to move off this project temporarily so without time to devote to solving this properly I've added a temporary solution to enable other team members to continue working.

moved: #210 (comment)

ciaranschutte · 2024-09-25T03:06:43Z

+	/* TODO: Remove Demo Data logic */
+	const demoFileMetadata: FileMetaData = {
+ objectId: 'demoFileData',
+ parts: [
+ {
+ url: 'https://s3.amazonaws.com/iobio/NA12878/NA12878.autsome.bam',
+ },
+ ],
+	};


move out of component

Moved: Move Demo Data to its own file
I was trying to centralize all Demo Data logic so that removal wouldn't require scouring several places, see discussion here:
#210 (comment)
Because the Demo Data logic relies on hooks and state in the parent component I had it all grouped in one block with the understanding it was only temporary.
I've moved the Component and metadata constant so they are not constantly redeclared but the remaining logic is still tied to the parent component.

I'd ask what about demoFileMetadata is tied to a hook or state? It looks like a regular object with static property values.

Excellent - moved everything into the Demo Button component. Just needed a day to sleep on it.
There's some logic to manage switching from the Demo file back to a real file that needed untangling but it should be figured out now.

demariadaniel · 2024-09-25T15:05:44Z

Missing await added: Add missing await

@ciaranschutte re: #210 (comment)

ciaranschutte

offline discussion of context of this feature branch - we are good to approve as is and improve upon later

demariadaniel · 2024-09-10T17:40:16Z

Loads selected file from Table Data in Iobio visualizer
Requests object URL from Score on table page load
Basic error handling for 0 files, multiple files, and wrong file types
Demo File URL for testing visualizer integration while demo files in Arranger are updated

Screen.Recording.2024-09-17.at.3.13.07.PM.mov

…feat/55-load-files

ciaranschutte · 2024-09-24T14:16:50Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


is this needed if there's this https://github.com/overture-stack/stage/pull/210/files#diff-d22d109917820bd9b112220302d1f28113412fb3db7c40ada3d03d835f5aacc1R30 typeguard in use?

It's because of tableData.find, which returns : unknown
tableData is unknown[] || []

This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

Updated in 33629a4
As discussed on Slack the .filter as would ideally not be needed but something about the type guard rowIsFileData isn't working quite as intended.

ciaranschutte · 2024-09-24T14:21:49Z

+export const baseScoreDownloadParams = {
+	external: 'true',
+	offset: '0',
+	'User-Agent': 'unknown',
+};


interesting note - these can be their actual types and not only strings because using URLSearchParams will format them correctly as needed

Good idea but URLSearchParams is actually expecting Record<string, string>

Where is the type coming from?
I checked this works:
newURLSearchParams({ext: true,count: 8,x: 'a'}).toString()
also check 2nd example here:
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#examples

Errors when updating types/values as described

Checked this again and seeing these same errors using the example you provided so this is a good one to compare notes on

ciaranschutte · 2024-09-24T23:21:29Z

 export const getScoreDownloadUrls = async (fileData: FileTableData) => {
 	const { NEXT_PUBLIC_SCORE_API_URL } = getConfig();
-	const length = fileData.file.size.toString();
+	const length = fileData.file?.size?.toString();


This is optional chaining
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
not nullish coalescing
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing
and will potentially return undefined which does not have the toString method

mentioning nullish coalescing because I think they're being confused in this PR 803ddab

I used the wrong term but optional chaining does protect against errors in this case. Tested in Node.
const test = null; test?.key?.data?.toString(); // returns undefined test.toString(); // throws error
I'll double check the logic. I added this to handle issues encountered with new demo data added yesterday. It was working at the time of testing.

ciaranschutte

few bits of React improvement:

regarding asyncs and this comment
#210 (comment)
I think you might need to follow the example down to this section in the link you shared: https://devtrium.com/posts/async-functions-useeffect#what-if-you-need-to-extract-the-function-outside-useeffect

smaller ones:

nested components not needed
no need for useMemo
static vars inside components

looks like the core bit of state we care about is the fileUrl so my gut is telling me that there's improvements we could make in computed that but I don't have bandwidth for that right now. I imagine it's along the lines of just passing in a fileUrl and loading props to the BamTable and leaving the reasoning about files somewhere else.

ciaranschutte · 2024-09-25T02:08:29Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

ciaranschutte · 2024-09-25T02:12:47Z

+	const response = await axios.get(
+ urlJoin(NEXT_PUBLIC_SCORE_API_URL, SCORE_API_DOWNLOAD_PATH, object_id, `?${urlParams}`),
+ {
+ headers: { accept: '*/*' },
+ },
+	);
+
+	if (response.status === 200) {
+ return response.data;
+	}
+	console.error(`Error at getScoreDownloadUrls with object_id ${object_id}`);
+	throw new Error(`Error at getScoreDownloadUrls status: ${response.status}, ok: false`);
+};


Does this work with errors?
I think there's an interceptor in components/utils that will Promise.reject(err)
but I don't think response will necessarily have a status field in this case

OK - axios was throwing an error, but it was printing to console, and not reading these lines.
I've straightened this out with Axios async/try/catch. Errors yield infinite loading state.

ciaranschutte · 2024-09-25T02:27:38Z

 );

-const BamTable = () => {
+const BamTable = ({ file }: { file: FileTableData | undefined }) => {


What happens if undefined is passed in as a prop?
I can get a local setup working tomorrow if I have time but seems like it might stay stuck on loading state and console logs an error - which isn't a working component

You are correct that is what happens. Right now this is managed by disabling the files/visualization button when the bamFile is undefined. There is no URL management for bamFile selected or FileTable/BamTable. File is undefined on page load.

ciaranschutte · 2024-09-25T02:30:05Z

+	const DemoDataButton = () => (
+ <div>
+ <button
+ css={css`
+ border: 2px solid ${theme.colors.accent};
+ border-radius: 5px;
+ min-width: fit-content;
+ padding: 3px 10px;
+ ${getToggleButtonStyles(isDemoData, theme)}
+ `}
+ onClick={loadDemoFile}
+ >
+ {isDemoData ? 'View File Data' : 'View Demo Data'}
+ </button>
+ </div>
+	);


I don't think we should necessarily be creating "demo" specific code.
But if we have to this is a component and should exist by itself and take props.
This isolates it significantly more from what is our production? real? code plus it's just correct to not have this be created on every render

I will revisit React / code quality comments. To clarify:
This is a feature branch deployed to its own URL on a project that is deprioritized for the next 3-6 months due to project scheduling requirements.
Demo specific code is provided due to the fact that the files currently hosted in Song/Arranger are not working with the Iobio server. This is a data compatibility issue that needs additional troubleshooting.
The Demo code is there to facilitate short term UI testing while the data issues are resolved. Without an option to load the demo data the Visualization page displays multiple NaN and blank graphs. So without the Demo data there's no testing the Iobio 'happy path', which does work.
I've been asked to move off this project temporarily so without time to devote to solving this properly I've added a temporary solution to enable other team members to continue working.

moved: #210 (comment)

ciaranschutte · 2024-09-25T03:06:43Z

+	/* TODO: Remove Demo Data logic */
+	const demoFileMetadata: FileMetaData = {
+ objectId: 'demoFileData',
+ parts: [
+ {
+ url: 'https://s3.amazonaws.com/iobio/NA12878/NA12878.autsome.bam',
+ },
+ ],
+	};


move out of component

Moved: Move Demo Data to its own file
I was trying to centralize all Demo Data logic so that removal wouldn't require scouring several places, see discussion here:
#210 (comment)
Because the Demo Data logic relies on hooks and state in the parent component I had it all grouped in one block with the understanding it was only temporary.
I've moved the Component and metadata constant so they are not constantly redeclared but the remaining logic is still tied to the parent component.

I'd ask what about demoFileMetadata is tied to a hook or state? It looks like a regular object with static property values.

Excellent - moved everything into the Demo Button component. Just needed a day to sleep on it.
There's some logic to manage switching from the Demo file back to a real file that needed untangling but it should be figured out now.

demariadaniel · 2024-09-25T15:05:44Z

Missing await added: Add missing await

@ciaranschutte re: #210 (comment)

ciaranschutte

offline discussion of context of this feature branch - we are good to approve as is and improve upon later

demariadaniel · 2024-09-10T17:40:16Z

Loads selected file from Table Data in Iobio visualizer
Requests object URL from Score on table page load
Basic error handling for 0 files, multiple files, and wrong file types
Demo File URL for testing visualizer integration while demo files in Arranger are updated

Screen.Recording.2024-09-17.at.3.13.07.PM.mov

…feat/55-load-files

ciaranschutte · 2024-09-24T14:16:50Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


is this needed if there's this https://github.com/overture-stack/stage/pull/210/files#diff-d22d109917820bd9b112220302d1f28113412fb3db7c40ada3d03d835f5aacc1R30 typeguard in use?

It's because of tableData.find, which returns : unknown
tableData is unknown[] || []

This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

Updated in 33629a4
As discussed on Slack the .filter as would ideally not be needed but something about the type guard rowIsFileData isn't working quite as intended.

ciaranschutte · 2024-09-24T14:21:49Z

+export const baseScoreDownloadParams = {
+	external: 'true',
+	offset: '0',
+	'User-Agent': 'unknown',
+};


interesting note - these can be their actual types and not only strings because using URLSearchParams will format them correctly as needed

Good idea but URLSearchParams is actually expecting Record<string, string>

Where is the type coming from?
I checked this works:
newURLSearchParams({ext: true,count: 8,x: 'a'}).toString()
also check 2nd example here:
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#examples

Errors when updating types/values as described

Checked this again and seeing these same errors using the example you provided so this is a good one to compare notes on

ciaranschutte · 2024-09-24T23:21:29Z

 export const getScoreDownloadUrls = async (fileData: FileTableData) => {
 	const { NEXT_PUBLIC_SCORE_API_URL } = getConfig();
-	const length = fileData.file.size.toString();
+	const length = fileData.file?.size?.toString();


This is optional chaining
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
not nullish coalescing
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing
and will potentially return undefined which does not have the toString method

mentioning nullish coalescing because I think they're being confused in this PR 803ddab

I used the wrong term but optional chaining does protect against errors in this case. Tested in Node.
const test = null; test?.key?.data?.toString(); // returns undefined test.toString(); // throws error
I'll double check the logic. I added this to handle issues encountered with new demo data added yesterday. It was working at the time of testing.

ciaranschutte

few bits of React improvement:

regarding asyncs and this comment
#210 (comment)
I think you might need to follow the example down to this section in the link you shared: https://devtrium.com/posts/async-functions-useeffect#what-if-you-need-to-extract-the-function-outside-useeffect

smaller ones:

nested components not needed
no need for useMemo
static vars inside components

looks like the core bit of state we care about is the fileUrl so my gut is telling me that there's improvements we could make in computed that but I don't have bandwidth for that right now. I imagine it's along the lines of just passing in a fileUrl and loading props to the BamTable and leaving the reasoning about files somewhere else.

ciaranschutte · 2024-09-25T02:08:29Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

ciaranschutte · 2024-09-25T02:12:47Z

+	const response = await axios.get(
+ urlJoin(NEXT_PUBLIC_SCORE_API_URL, SCORE_API_DOWNLOAD_PATH, object_id, `?${urlParams}`),
+ {
+ headers: { accept: '*/*' },
+ },
+	);
+
+	if (response.status === 200) {
+ return response.data;
+	}
+	console.error(`Error at getScoreDownloadUrls with object_id ${object_id}`);
+	throw new Error(`Error at getScoreDownloadUrls status: ${response.status}, ok: false`);
+};


Does this work with errors?
I think there's an interceptor in components/utils that will Promise.reject(err)
but I don't think response will necessarily have a status field in this case

OK - axios was throwing an error, but it was printing to console, and not reading these lines.
I've straightened this out with Axios async/try/catch. Errors yield infinite loading state.

ciaranschutte · 2024-09-25T02:27:38Z

 );

-const BamTable = () => {
+const BamTable = ({ file }: { file: FileTableData | undefined }) => {


What happens if undefined is passed in as a prop?
I can get a local setup working tomorrow if I have time but seems like it might stay stuck on loading state and console logs an error - which isn't a working component

You are correct that is what happens. Right now this is managed by disabling the files/visualization button when the bamFile is undefined. There is no URL management for bamFile selected or FileTable/BamTable. File is undefined on page load.

ciaranschutte · 2024-09-25T02:30:05Z

+	const DemoDataButton = () => (
+ <div>
+ <button
+ css={css`
+ border: 2px solid ${theme.colors.accent};
+ border-radius: 5px;
+ min-width: fit-content;
+ padding: 3px 10px;
+ ${getToggleButtonStyles(isDemoData, theme)}
+ `}
+ onClick={loadDemoFile}
+ >
+ {isDemoData ? 'View File Data' : 'View Demo Data'}
+ </button>
+ </div>
+	);


I don't think we should necessarily be creating "demo" specific code.
But if we have to this is a component and should exist by itself and take props.
This isolates it significantly more from what is our production? real? code plus it's just correct to not have this be created on every render

I will revisit React / code quality comments. To clarify:
This is a feature branch deployed to its own URL on a project that is deprioritized for the next 3-6 months due to project scheduling requirements.
Demo specific code is provided due to the fact that the files currently hosted in Song/Arranger are not working with the Iobio server. This is a data compatibility issue that needs additional troubleshooting.
The Demo code is there to facilitate short term UI testing while the data issues are resolved. Without an option to load the demo data the Visualization page displays multiple NaN and blank graphs. So without the Demo data there's no testing the Iobio 'happy path', which does work.
I've been asked to move off this project temporarily so without time to devote to solving this properly I've added a temporary solution to enable other team members to continue working.

moved: #210 (comment)

ciaranschutte · 2024-09-25T03:06:43Z

+	/* TODO: Remove Demo Data logic */
+	const demoFileMetadata: FileMetaData = {
+ objectId: 'demoFileData',
+ parts: [
+ {
+ url: 'https://s3.amazonaws.com/iobio/NA12878/NA12878.autsome.bam',
+ },
+ ],
+	};


move out of component

Moved: Move Demo Data to its own file
I was trying to centralize all Demo Data logic so that removal wouldn't require scouring several places, see discussion here:
#210 (comment)
Because the Demo Data logic relies on hooks and state in the parent component I had it all grouped in one block with the understanding it was only temporary.
I've moved the Component and metadata constant so they are not constantly redeclared but the remaining logic is still tied to the parent component.

I'd ask what about demoFileMetadata is tied to a hook or state? It looks like a regular object with static property values.

Excellent - moved everything into the Demo Button component. Just needed a day to sleep on it.
There's some logic to manage switching from the Demo file back to a real file that needed untangling but it should be figured out now.

demariadaniel · 2024-09-25T15:05:44Z

Missing await added: Add missing await

@ciaranschutte re: #210 (comment)

ciaranschutte

offline discussion of context of this feature branch - we are good to approve as is and improve upon later

demariadaniel · 2024-09-10T17:40:16Z

Loads selected file from Table Data in Iobio visualizer
Requests object URL from Score on table page load
Basic error handling for 0 files, multiple files, and wrong file types
Demo File URL for testing visualizer integration while demo files in Arranger are updated

Screen.Recording.2024-09-17.at.3.13.07.PM.mov

…feat/55-load-files

ciaranschutte · 2024-09-24T14:16:50Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


is this needed if there's this https://github.com/overture-stack/stage/pull/210/files#diff-d22d109917820bd9b112220302d1f28113412fb3db7c40ada3d03d835f5aacc1R30 typeguard in use?

It's because of tableData.find, which returns : unknown
tableData is unknown[] || []

This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

Updated in 33629a4
As discussed on Slack the .filter as would ideally not be needed but something about the type guard rowIsFileData isn't working quite as intended.

ciaranschutte · 2024-09-24T14:21:49Z

+export const baseScoreDownloadParams = {
+	external: 'true',
+	offset: '0',
+	'User-Agent': 'unknown',
+};


interesting note - these can be their actual types and not only strings because using URLSearchParams will format them correctly as needed

Good idea but URLSearchParams is actually expecting Record<string, string>

Where is the type coming from?
I checked this works:
newURLSearchParams({ext: true,count: 8,x: 'a'}).toString()
also check 2nd example here:
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#examples

Errors when updating types/values as described

Checked this again and seeing these same errors using the example you provided so this is a good one to compare notes on

ciaranschutte · 2024-09-24T23:21:29Z

 export const getScoreDownloadUrls = async (fileData: FileTableData) => {
 	const { NEXT_PUBLIC_SCORE_API_URL } = getConfig();
-	const length = fileData.file.size.toString();
+	const length = fileData.file?.size?.toString();


This is optional chaining
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
not nullish coalescing
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing
and will potentially return undefined which does not have the toString method

mentioning nullish coalescing because I think they're being confused in this PR 803ddab

I used the wrong term but optional chaining does protect against errors in this case. Tested in Node.
const test = null; test?.key?.data?.toString(); // returns undefined test.toString(); // throws error
I'll double check the logic. I added this to handle issues encountered with new demo data added yesterday. It was working at the time of testing.

ciaranschutte

few bits of React improvement:

regarding asyncs and this comment
#210 (comment)
I think you might need to follow the example down to this section in the link you shared: https://devtrium.com/posts/async-functions-useeffect#what-if-you-need-to-extract-the-function-outside-useeffect

smaller ones:

nested components not needed
no need for useMemo
static vars inside components

looks like the core bit of state we care about is the fileUrl so my gut is telling me that there's improvements we could make in computed that but I don't have bandwidth for that right now. I imagine it's along the lines of just passing in a fileUrl and loading props to the BamTable and leaving the reasoning about files somewhere else.

ciaranschutte · 2024-09-25T02:08:29Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

ciaranschutte · 2024-09-25T02:12:47Z

+	const response = await axios.get(
+ urlJoin(NEXT_PUBLIC_SCORE_API_URL, SCORE_API_DOWNLOAD_PATH, object_id, `?${urlParams}`),
+ {
+ headers: { accept: '*/*' },
+ },
+	);
+
+	if (response.status === 200) {
+ return response.data;
+	}
+	console.error(`Error at getScoreDownloadUrls with object_id ${object_id}`);
+	throw new Error(`Error at getScoreDownloadUrls status: ${response.status}, ok: false`);
+};


Does this work with errors?
I think there's an interceptor in components/utils that will Promise.reject(err)
but I don't think response will necessarily have a status field in this case

OK - axios was throwing an error, but it was printing to console, and not reading these lines.
I've straightened this out with Axios async/try/catch. Errors yield infinite loading state.

ciaranschutte · 2024-09-25T02:27:38Z

 );

-const BamTable = () => {
+const BamTable = ({ file }: { file: FileTableData | undefined }) => {


What happens if undefined is passed in as a prop?
I can get a local setup working tomorrow if I have time but seems like it might stay stuck on loading state and console logs an error - which isn't a working component

You are correct that is what happens. Right now this is managed by disabling the files/visualization button when the bamFile is undefined. There is no URL management for bamFile selected or FileTable/BamTable. File is undefined on page load.

ciaranschutte · 2024-09-25T02:30:05Z

+	const DemoDataButton = () => (
+ <div>
+ <button
+ css={css`
+ border: 2px solid ${theme.colors.accent};
+ border-radius: 5px;
+ min-width: fit-content;
+ padding: 3px 10px;
+ ${getToggleButtonStyles(isDemoData, theme)}
+ `}
+ onClick={loadDemoFile}
+ >
+ {isDemoData ? 'View File Data' : 'View Demo Data'}
+ </button>
+ </div>
+	);


I don't think we should necessarily be creating "demo" specific code.
But if we have to this is a component and should exist by itself and take props.
This isolates it significantly more from what is our production? real? code plus it's just correct to not have this be created on every render

I will revisit React / code quality comments. To clarify:
This is a feature branch deployed to its own URL on a project that is deprioritized for the next 3-6 months due to project scheduling requirements.
Demo specific code is provided due to the fact that the files currently hosted in Song/Arranger are not working with the Iobio server. This is a data compatibility issue that needs additional troubleshooting.
The Demo code is there to facilitate short term UI testing while the data issues are resolved. Without an option to load the demo data the Visualization page displays multiple NaN and blank graphs. So without the Demo data there's no testing the Iobio 'happy path', which does work.
I've been asked to move off this project temporarily so without time to devote to solving this properly I've added a temporary solution to enable other team members to continue working.

moved: #210 (comment)

ciaranschutte · 2024-09-25T03:06:43Z

+	/* TODO: Remove Demo Data logic */
+	const demoFileMetadata: FileMetaData = {
+ objectId: 'demoFileData',
+ parts: [
+ {
+ url: 'https://s3.amazonaws.com/iobio/NA12878/NA12878.autsome.bam',
+ },
+ ],
+	};


move out of component

Moved: Move Demo Data to its own file
I was trying to centralize all Demo Data logic so that removal wouldn't require scouring several places, see discussion here:
#210 (comment)
Because the Demo Data logic relies on hooks and state in the parent component I had it all grouped in one block with the understanding it was only temporary.
I've moved the Component and metadata constant so they are not constantly redeclared but the remaining logic is still tied to the parent component.

I'd ask what about demoFileMetadata is tied to a hook or state? It looks like a regular object with static property values.

Excellent - moved everything into the Demo Button component. Just needed a day to sleep on it.
There's some logic to manage switching from the Demo file back to a real file that needed untangling but it should be figured out now.

demariadaniel · 2024-09-25T15:05:44Z

Missing await added: Add missing await

@ciaranschutte re: #210 (comment)

ciaranschutte

offline discussion of context of this feature branch - we are good to approve as is and improve upon later

demariadaniel · 2024-09-10T17:40:16Z

Loads selected file from Table Data in Iobio visualizer
Requests object URL from Score on table page load
Basic error handling for 0 files, multiple files, and wrong file types
Demo File URL for testing visualizer integration while demo files in Arranger are updated

Screen.Recording.2024-09-17.at.3.13.07.PM.mov

…feat/55-load-files

ciaranschutte · 2024-09-24T14:16:50Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


is this needed if there's this https://github.com/overture-stack/stage/pull/210/files#diff-d22d109917820bd9b112220302d1f28113412fb3db7c40ada3d03d835f5aacc1R30 typeguard in use?

It's because of tableData.find, which returns : unknown
tableData is unknown[] || []

This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

Updated in 33629a4
As discussed on Slack the .filter as would ideally not be needed but something about the type guard rowIsFileData isn't working quite as intended.

ciaranschutte · 2024-09-24T14:21:49Z

+export const baseScoreDownloadParams = {
+	external: 'true',
+	offset: '0',
+	'User-Agent': 'unknown',
+};


interesting note - these can be their actual types and not only strings because using URLSearchParams will format them correctly as needed

Good idea but URLSearchParams is actually expecting Record<string, string>

Where is the type coming from?
I checked this works:
newURLSearchParams({ext: true,count: 8,x: 'a'}).toString()
also check 2nd example here:
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#examples

Errors when updating types/values as described

Checked this again and seeing these same errors using the example you provided so this is a good one to compare notes on

ciaranschutte · 2024-09-24T23:21:29Z

 export const getScoreDownloadUrls = async (fileData: FileTableData) => {
 	const { NEXT_PUBLIC_SCORE_API_URL } = getConfig();
-	const length = fileData.file.size.toString();
+	const length = fileData.file?.size?.toString();


This is optional chaining
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
not nullish coalescing
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing
and will potentially return undefined which does not have the toString method

mentioning nullish coalescing because I think they're being confused in this PR 803ddab

I used the wrong term but optional chaining does protect against errors in this case. Tested in Node.
const test = null; test?.key?.data?.toString(); // returns undefined test.toString(); // throws error
I'll double check the logic. I added this to handle issues encountered with new demo data added yesterday. It was working at the time of testing.

ciaranschutte

few bits of React improvement:

regarding asyncs and this comment
#210 (comment)
I think you might need to follow the example down to this section in the link you shared: https://devtrium.com/posts/async-functions-useeffect#what-if-you-need-to-extract-the-function-outside-useeffect

smaller ones:

nested components not needed
no need for useMemo
static vars inside components

looks like the core bit of state we care about is the fileUrl so my gut is telling me that there's improvements we could make in computed that but I don't have bandwidth for that right now. I imagine it's along the lines of just passing in a fileUrl and loading props to the BamTable and leaving the reasoning about files somewhere else.

ciaranschutte · 2024-09-25T02:08:29Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

ciaranschutte · 2024-09-25T02:12:47Z

+	const response = await axios.get(
+ urlJoin(NEXT_PUBLIC_SCORE_API_URL, SCORE_API_DOWNLOAD_PATH, object_id, `?${urlParams}`),
+ {
+ headers: { accept: '*/*' },
+ },
+	);
+
+	if (response.status === 200) {
+ return response.data;
+	}
+	console.error(`Error at getScoreDownloadUrls with object_id ${object_id}`);
+	throw new Error(`Error at getScoreDownloadUrls status: ${response.status}, ok: false`);
+};


Does this work with errors?
I think there's an interceptor in components/utils that will Promise.reject(err)
but I don't think response will necessarily have a status field in this case

OK - axios was throwing an error, but it was printing to console, and not reading these lines.
I've straightened this out with Axios async/try/catch. Errors yield infinite loading state.

ciaranschutte · 2024-09-25T02:27:38Z

 );

-const BamTable = () => {
+const BamTable = ({ file }: { file: FileTableData | undefined }) => {


What happens if undefined is passed in as a prop?
I can get a local setup working tomorrow if I have time but seems like it might stay stuck on loading state and console logs an error - which isn't a working component

You are correct that is what happens. Right now this is managed by disabling the files/visualization button when the bamFile is undefined. There is no URL management for bamFile selected or FileTable/BamTable. File is undefined on page load.

ciaranschutte · 2024-09-25T02:30:05Z

+	const DemoDataButton = () => (
+ <div>
+ <button
+ css={css`
+ border: 2px solid ${theme.colors.accent};
+ border-radius: 5px;
+ min-width: fit-content;
+ padding: 3px 10px;
+ ${getToggleButtonStyles(isDemoData, theme)}
+ `}
+ onClick={loadDemoFile}
+ >
+ {isDemoData ? 'View File Data' : 'View Demo Data'}
+ </button>
+ </div>
+	);


I don't think we should necessarily be creating "demo" specific code.
But if we have to this is a component and should exist by itself and take props.
This isolates it significantly more from what is our production? real? code plus it's just correct to not have this be created on every render

I will revisit React / code quality comments. To clarify:
This is a feature branch deployed to its own URL on a project that is deprioritized for the next 3-6 months due to project scheduling requirements.
Demo specific code is provided due to the fact that the files currently hosted in Song/Arranger are not working with the Iobio server. This is a data compatibility issue that needs additional troubleshooting.
The Demo code is there to facilitate short term UI testing while the data issues are resolved. Without an option to load the demo data the Visualization page displays multiple NaN and blank graphs. So without the Demo data there's no testing the Iobio 'happy path', which does work.
I've been asked to move off this project temporarily so without time to devote to solving this properly I've added a temporary solution to enable other team members to continue working.

moved: #210 (comment)

ciaranschutte · 2024-09-25T03:06:43Z

+	/* TODO: Remove Demo Data logic */
+	const demoFileMetadata: FileMetaData = {
+ objectId: 'demoFileData',
+ parts: [
+ {
+ url: 'https://s3.amazonaws.com/iobio/NA12878/NA12878.autsome.bam',
+ },
+ ],
+	};


move out of component

Moved: Move Demo Data to its own file
I was trying to centralize all Demo Data logic so that removal wouldn't require scouring several places, see discussion here:
#210 (comment)
Because the Demo Data logic relies on hooks and state in the parent component I had it all grouped in one block with the understanding it was only temporary.
I've moved the Component and metadata constant so they are not constantly redeclared but the remaining logic is still tied to the parent component.

I'd ask what about demoFileMetadata is tied to a hook or state? It looks like a regular object with static property values.

Excellent - moved everything into the Demo Button component. Just needed a day to sleep on it.
There's some logic to manage switching from the Demo file back to a real file that needed untangling but it should be figured out now.

demariadaniel · 2024-09-25T15:05:44Z

Missing await added: Add missing await

@ciaranschutte re: #210 (comment)

ciaranschutte

offline discussion of context of this feature branch - we are good to approve as is and improve upon later

demariadaniel · 2024-09-10T17:40:16Z

Loads selected file from Table Data in Iobio visualizer
Requests object URL from Score on table page load
Basic error handling for 0 files, multiple files, and wrong file types
Demo File URL for testing visualizer integration while demo files in Arranger are updated

Screen.Recording.2024-09-17.at.3.13.07.PM.mov

…feat/55-load-files

ciaranschutte · 2024-09-24T14:16:50Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


is this needed if there's this https://github.com/overture-stack/stage/pull/210/files#diff-d22d109917820bd9b112220302d1f28113412fb3db7c40ada3d03d835f5aacc1R30 typeguard in use?

It's because of tableData.find, which returns : unknown
tableData is unknown[] || []

This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

Updated in 33629a4
As discussed on Slack the .filter as would ideally not be needed but something about the type guard rowIsFileData isn't working quite as intended.

ciaranschutte · 2024-09-24T14:21:49Z

+export const baseScoreDownloadParams = {
+	external: 'true',
+	offset: '0',
+	'User-Agent': 'unknown',
+};


interesting note - these can be their actual types and not only strings because using URLSearchParams will format them correctly as needed

Good idea but URLSearchParams is actually expecting Record<string, string>

Where is the type coming from?
I checked this works:
newURLSearchParams({ext: true,count: 8,x: 'a'}).toString()
also check 2nd example here:
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#examples

Errors when updating types/values as described

Checked this again and seeing these same errors using the example you provided so this is a good one to compare notes on

ciaranschutte · 2024-09-24T23:21:29Z

 export const getScoreDownloadUrls = async (fileData: FileTableData) => {
 	const { NEXT_PUBLIC_SCORE_API_URL } = getConfig();
-	const length = fileData.file.size.toString();
+	const length = fileData.file?.size?.toString();


This is optional chaining
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
not nullish coalescing
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing
and will potentially return undefined which does not have the toString method

mentioning nullish coalescing because I think they're being confused in this PR 803ddab

I used the wrong term but optional chaining does protect against errors in this case. Tested in Node.
const test = null; test?.key?.data?.toString(); // returns undefined test.toString(); // throws error
I'll double check the logic. I added this to handle issues encountered with new demo data added yesterday. It was working at the time of testing.

ciaranschutte

few bits of React improvement:

regarding asyncs and this comment
#210 (comment)
I think you might need to follow the example down to this section in the link you shared: https://devtrium.com/posts/async-functions-useeffect#what-if-you-need-to-extract-the-function-outside-useeffect

smaller ones:

nested components not needed
no need for useMemo
static vars inside components

looks like the core bit of state we care about is the fileUrl so my gut is telling me that there's improvements we could make in computed that but I don't have bandwidth for that right now. I imagine it's along the lines of just passing in a fileUrl and loading props to the BamTable and leaving the reasoning about files somewhere else.

ciaranschutte · 2024-09-25T02:08:29Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

ciaranschutte · 2024-09-25T02:12:47Z

+	const response = await axios.get(
+ urlJoin(NEXT_PUBLIC_SCORE_API_URL, SCORE_API_DOWNLOAD_PATH, object_id, `?${urlParams}`),
+ {
+ headers: { accept: '*/*' },
+ },
+	);
+
+	if (response.status === 200) {
+ return response.data;
+	}
+	console.error(`Error at getScoreDownloadUrls with object_id ${object_id}`);
+	throw new Error(`Error at getScoreDownloadUrls status: ${response.status}, ok: false`);
+};


Does this work with errors?
I think there's an interceptor in components/utils that will Promise.reject(err)
but I don't think response will necessarily have a status field in this case

OK - axios was throwing an error, but it was printing to console, and not reading these lines.
I've straightened this out with Axios async/try/catch. Errors yield infinite loading state.

ciaranschutte · 2024-09-25T02:27:38Z

 );

-const BamTable = () => {
+const BamTable = ({ file }: { file: FileTableData | undefined }) => {


What happens if undefined is passed in as a prop?
I can get a local setup working tomorrow if I have time but seems like it might stay stuck on loading state and console logs an error - which isn't a working component

You are correct that is what happens. Right now this is managed by disabling the files/visualization button when the bamFile is undefined. There is no URL management for bamFile selected or FileTable/BamTable. File is undefined on page load.

ciaranschutte · 2024-09-25T02:30:05Z

+	const DemoDataButton = () => (
+ <div>
+ <button
+ css={css`
+ border: 2px solid ${theme.colors.accent};
+ border-radius: 5px;
+ min-width: fit-content;
+ padding: 3px 10px;
+ ${getToggleButtonStyles(isDemoData, theme)}
+ `}
+ onClick={loadDemoFile}
+ >
+ {isDemoData ? 'View File Data' : 'View Demo Data'}
+ </button>
+ </div>
+	);


I don't think we should necessarily be creating "demo" specific code.
But if we have to this is a component and should exist by itself and take props.
This isolates it significantly more from what is our production? real? code plus it's just correct to not have this be created on every render

I will revisit React / code quality comments. To clarify:
This is a feature branch deployed to its own URL on a project that is deprioritized for the next 3-6 months due to project scheduling requirements.
Demo specific code is provided due to the fact that the files currently hosted in Song/Arranger are not working with the Iobio server. This is a data compatibility issue that needs additional troubleshooting.
The Demo code is there to facilitate short term UI testing while the data issues are resolved. Without an option to load the demo data the Visualization page displays multiple NaN and blank graphs. So without the Demo data there's no testing the Iobio 'happy path', which does work.
I've been asked to move off this project temporarily so without time to devote to solving this properly I've added a temporary solution to enable other team members to continue working.

moved: #210 (comment)

ciaranschutte · 2024-09-25T03:06:43Z

+	/* TODO: Remove Demo Data logic */
+	const demoFileMetadata: FileMetaData = {
+ objectId: 'demoFileData',
+ parts: [
+ {
+ url: 'https://s3.amazonaws.com/iobio/NA12878/NA12878.autsome.bam',
+ },
+ ],
+	};


move out of component

Moved: Move Demo Data to its own file
I was trying to centralize all Demo Data logic so that removal wouldn't require scouring several places, see discussion here:
#210 (comment)
Because the Demo Data logic relies on hooks and state in the parent component I had it all grouped in one block with the understanding it was only temporary.
I've moved the Component and metadata constant so they are not constantly redeclared but the remaining logic is still tied to the parent component.

I'd ask what about demoFileMetadata is tied to a hook or state? It looks like a regular object with static property values.

Excellent - moved everything into the Demo Button component. Just needed a day to sleep on it.
There's some logic to manage switching from the Demo file back to a real file that needed untangling but it should be figured out now.

demariadaniel · 2024-09-25T15:05:44Z

Missing await added: Add missing await

@ciaranschutte re: #210 (comment)

ciaranschutte

offline discussion of context of this feature branch - we are good to approve as is and improve upon later

demariadaniel · 2024-09-10T17:40:16Z

Loads selected file from Table Data in Iobio visualizer
Requests object URL from Score on table page load
Basic error handling for 0 files, multiple files, and wrong file types
Demo File URL for testing visualizer integration while demo files in Arranger are updated

Screen.Recording.2024-09-17.at.3.13.07.PM.mov

…feat/55-load-files

ciaranschutte · 2024-09-24T14:16:50Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


is this needed if there's this https://github.com/overture-stack/stage/pull/210/files#diff-d22d109917820bd9b112220302d1f28113412fb3db7c40ada3d03d835f5aacc1R30 typeguard in use?

It's because of tableData.find, which returns : unknown
tableData is unknown[] || []

This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

Updated in 33629a4
As discussed on Slack the .filter as would ideally not be needed but something about the type guard rowIsFileData isn't working quite as intended.

ciaranschutte · 2024-09-24T14:21:49Z

+export const baseScoreDownloadParams = {
+	external: 'true',
+	offset: '0',
+	'User-Agent': 'unknown',
+};


interesting note - these can be their actual types and not only strings because using URLSearchParams will format them correctly as needed

Good idea but URLSearchParams is actually expecting Record<string, string>

Where is the type coming from?
I checked this works:
newURLSearchParams({ext: true,count: 8,x: 'a'}).toString()
also check 2nd example here:
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#examples

Errors when updating types/values as described

Checked this again and seeing these same errors using the example you provided so this is a good one to compare notes on

ciaranschutte · 2024-09-24T23:21:29Z

 export const getScoreDownloadUrls = async (fileData: FileTableData) => {
 	const { NEXT_PUBLIC_SCORE_API_URL } = getConfig();
-	const length = fileData.file.size.toString();
+	const length = fileData.file?.size?.toString();


This is optional chaining
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
not nullish coalescing
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing
and will potentially return undefined which does not have the toString method

mentioning nullish coalescing because I think they're being confused in this PR 803ddab

I used the wrong term but optional chaining does protect against errors in this case. Tested in Node.
const test = null; test?.key?.data?.toString(); // returns undefined test.toString(); // throws error
I'll double check the logic. I added this to handle issues encountered with new demo data added yesterday. It was working at the time of testing.

ciaranschutte

few bits of React improvement:

regarding asyncs and this comment
#210 (comment)
I think you might need to follow the example down to this section in the link you shared: https://devtrium.com/posts/async-functions-useeffect#what-if-you-need-to-extract-the-function-outside-useeffect

smaller ones:

nested components not needed
no need for useMemo
static vars inside components

looks like the core bit of state we care about is the fileUrl so my gut is telling me that there's improvements we could make in computed that but I don't have bandwidth for that right now. I imagine it's along the lines of just passing in a fileUrl and loading props to the BamTable and leaving the reasoning about files somewhere else.

ciaranschutte · 2024-09-25T02:08:29Z

+ const isBamFile = file_type && BamFileExtensions.includes(file_type);
+ return idMatch && isBamFile;
+ }
+ }) as FileTableData | undefined;


This confuses me. there's a rowIsFileData typeguard for tableData (the element in the array)
So surely tableData (same name, but outer closure) is just an array of that kind? Reads like we know the type but just aren't using it.
Would type narrowing earlier work?

ciaranschutte · 2024-09-25T02:12:47Z

+	const response = await axios.get(
+ urlJoin(NEXT_PUBLIC_SCORE_API_URL, SCORE_API_DOWNLOAD_PATH, object_id, `?${urlParams}`),
+ {
+ headers: { accept: '*/*' },
+ },
+	);
+
+	if (response.status === 200) {
+ return response.data;
+	}
+	console.error(`Error at getScoreDownloadUrls with object_id ${object_id}`);
+	throw new Error(`Error at getScoreDownloadUrls status: ${response.status}, ok: false`);
+};


Does this work with errors?
I think there's an interceptor in components/utils that will Promise.reject(err)
but I don't think response will necessarily have a status field in this case

OK - axios was throwing an error, but it was printing to console, and not reading these lines.
I've straightened this out with Axios async/try/catch. Errors yield infinite loading state.

ciaranschutte · 2024-09-25T02:27:38Z

 );

-const BamTable = () => {
+const BamTable = ({ file }: { file: FileTableData | undefined }) => {


What happens if undefined is passed in as a prop?
I can get a local setup working tomorrow if I have time but seems like it might stay stuck on loading state and console logs an error - which isn't a working component

You are correct that is what happens. Right now this is managed by disabling the files/visualization button when the bamFile is undefined. There is no URL management for bamFile selected or FileTable/BamTable. File is undefined on page load.

ciaranschutte · 2024-09-25T02:30:05Z

+	const DemoDataButton = () => (
+ <div>
+ <button
+ css={css`
+ border: 2px solid ${theme.colors.accent};
+ border-radius: 5px;
+ min-width: fit-content;
+ padding: 3px 10px;
+ ${getToggleButtonStyles(isDemoData, theme)}
+ `}
+ onClick={loadDemoFile}
+ >
+ {isDemoData ? 'View File Data' : 'View Demo Data'}
+ </button>
+ </div>
+	);


I don't think we should necessarily be creating "demo" specific code.
But if we have to this is a component and should exist by itself and take props.
This isolates it significantly more from what is our production? real? code plus it's just correct to not have this be created on every render

I will revisit React / code quality comments. To clarify:
This is a feature branch deployed to its own URL on a project that is deprioritized for the next 3-6 months due to project scheduling requirements.
Demo specific code is provided due to the fact that the files currently hosted in Song/Arranger are not working with the Iobio server. This is a data compatibility issue that needs additional troubleshooting.
The Demo code is there to facilitate short term UI testing while the data issues are resolved. Without an option to load the demo data the Visualization page displays multiple NaN and blank graphs. So without the Demo data there's no testing the Iobio 'happy path', which does work.
I've been asked to move off this project temporarily so without time to devote to solving this properly I've added a temporary solution to enable other team members to continue working.

moved: #210 (comment)

ciaranschutte · 2024-09-25T03:06:43Z

+	/* TODO: Remove Demo Data logic */
+	const demoFileMetadata: FileMetaData = {
+ objectId: 'demoFileData',
+ parts: [
+ {
+ url: 'https://s3.amazonaws.com/iobio/NA12878/NA12878.autsome.bam',
+ },
+ ],
+	};


move out of component

Moved: Move Demo Data to its own file
I was trying to centralize all Demo Data logic so that removal wouldn't require scouring several places, see discussion here:
#210 (comment)
Because the Demo Data logic relies on hooks and state in the parent component I had it all grouped in one block with the understanding it was only temporary.
I've moved the Component and metadata constant so they are not constantly redeclared but the remaining logic is still tied to the parent component.

I'd ask what about demoFileMetadata is tied to a hook or state? It looks like a regular object with static property values.

Excellent - moved everything into the Demo Button component. Just needed a day to sleep on it.
There's some logic to manage switching from the Demo file back to a real file that needed untangling but it should be figured out now.

demariadaniel · 2024-09-25T15:05:44Z

Missing await added: Add missing await

@ciaranschutte re: #210 (comment)

ciaranschutte

offline discussion of context of this feature branch - we are good to approve as is and improve upon later

demariadaniel added 30 commits August 30, 2024 16:32

Buttons + first Layout
45644af

Tweak Chart size
ebbb96a

Adjust Container CSS
164200a

Update File Name
a26800b

Toggle Button CSS
41bf9ec

Toggle state
54389c0

Table Markup / CSS
c1bf667

Updated Button colors w/ Icon placeholder
2b5e07c

Style var cleanup
e042f17

Display: flex / use flex-start
25e25b4

Missing read coverage toggle
bdaa6e1

Import context from Iobio Library
f78f6eb

Add Icons
b7b1224

Reusable Functions + Variables
f340ce4

JSX rule names
42004d0

Copyright corrections
752853c

Latest iobio package
d90787d

isFileTableActive var name
ccd33b3

Remove Excess Fragment
b918ee2

Remove empty line
f3c38e9

Remove extra fragment (bam table)
c687f57

Reorg imports
85ec50a

Merge branch 'iobio' ofhttps://github.com/overture-stack/stageinto …
c18e72d
…feat/55-load-files

Context Consumer / SelectedRows
cf85bd7

Get Selected Bam File
b327e49

Download URL Function + Config
f92b78f

Extend error message
e59e9fb

Draft Score Download
99d17bd

Testing State
5955cbe

Add Metadata Type
0e3c995

Updated Async / Import
e738a23

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

justincorrigible reviewed Sep 24, 2024
View reviewed changes

Comment threadcomponents/pages/explorer/fileUtils.ts Outdated

Null handling + remove extra 'if'
0f6d20a

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

ciaranschutte suggested changes Sep 25, 2024
View reviewed changes

demariadaniel added 4 commits September 25, 2024 10:36

Score Url env.schema
9946bb0

Move Demo Data to its own file
fddb126

Add missing await
7cec0c6

Remove useMemo
880c9f0

Rename setElementState
7149feb

justincorrigible reviewed Sep 25, 2024
View reviewed changes

Comment thread.env.schema

demariadaniel added 12 commits September 25, 2024 11:20

Add env schema comment
10019ad

Remove Unused useRouter
12570a9

Change undefined syntax
e972f32

Change Axios Promise handling
1d96ef0

Return axios promise
282bc7d

Remove unused await
1cdce40

Prefer async for consistency
d1abf7a

Fix catch syntax
c79f08d

Remove error testing
4dd9e22

Type Narrowing
33629a4

Move Demo File function
f52fbb4

Update Loading Hook
5872309

ciaranschutte self-requested a review September 27, 2024 23:42

ciaranschutte approved these changes Sep 27, 2024
View reviewed changes

demariadaniel merged commit 484c2c0 into iobioSep 30, 2024

demariadaniel deleted the feat/55-load-files branch September 30, 2024 13:47

demariadaniel added 30 commits August 30, 2024 16:32

Buttons + first Layout
45644af

Tweak Chart size
ebbb96a

Adjust Container CSS
164200a

Update File Name
a26800b

Toggle Button CSS
41bf9ec

Toggle state
54389c0

Table Markup / CSS
c1bf667

Updated Button colors w/ Icon placeholder
2b5e07c

Style var cleanup
e042f17

Display: flex / use flex-start
25e25b4

Missing read coverage toggle
bdaa6e1

Import context from Iobio Library
f78f6eb

Add Icons
b7b1224

Reusable Functions + Variables
f340ce4

JSX rule names
42004d0

Copyright corrections
752853c

Latest iobio package
d90787d

isFileTableActive var name
ccd33b3

Remove Excess Fragment
b918ee2

Remove empty line
f3c38e9

Remove extra fragment (bam table)
c687f57

Reorg imports
85ec50a

Merge branch 'iobio' ofhttps://github.com/overture-stack/stageinto …
c18e72d
…feat/55-load-files

Context Consumer / SelectedRows
cf85bd7

Get Selected Bam File
b327e49

Download URL Function + Config
f92b78f

Extend error message
e59e9fb

Draft Score Download
99d17bd

Testing State
5955cbe

Add Metadata Type
0e3c995

Updated Async / Import
e738a23

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

justincorrigible reviewed Sep 24, 2024
View reviewed changes

Comment threadcomponents/pages/explorer/fileUtils.ts Outdated

Null handling + remove extra 'if'
0f6d20a

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

ciaranschutte suggested changes Sep 25, 2024
View reviewed changes

demariadaniel added 4 commits September 25, 2024 10:36

Score Url env.schema
9946bb0

Move Demo Data to its own file
fddb126

Add missing await
7cec0c6

Remove useMemo
880c9f0

Rename setElementState
7149feb

justincorrigible reviewed Sep 25, 2024
View reviewed changes

Comment thread.env.schema

demariadaniel added 12 commits September 25, 2024 11:20

Add env schema comment
10019ad

Remove Unused useRouter
12570a9

Change undefined syntax
e972f32

Change Axios Promise handling
1d96ef0

Return axios promise
282bc7d

Remove unused await
1cdce40

Prefer async for consistency
d1abf7a

Fix catch syntax
c79f08d

Remove error testing
4dd9e22

Type Narrowing
33629a4

Move Demo File function
f52fbb4

Update Loading Hook
5872309

ciaranschutte self-requested a review September 27, 2024 23:42

ciaranschutte approved these changes Sep 27, 2024
View reviewed changes

demariadaniel merged commit 484c2c0 into iobioSep 30, 2024

demariadaniel deleted the feat/55-load-files branch September 30, 2024 13:47

demariadaniel added 30 commits August 30, 2024 16:32

Buttons + first Layout
45644af

Tweak Chart size
ebbb96a

Adjust Container CSS
164200a

Update File Name
a26800b

Toggle Button CSS
41bf9ec

Toggle state
54389c0

Table Markup / CSS
c1bf667

Updated Button colors w/ Icon placeholder
2b5e07c

Style var cleanup
e042f17

Display: flex / use flex-start
25e25b4

Missing read coverage toggle
bdaa6e1

Import context from Iobio Library
f78f6eb

Add Icons
b7b1224

Reusable Functions + Variables
f340ce4

JSX rule names
42004d0

Copyright corrections
752853c

Latest iobio package
d90787d

isFileTableActive var name
ccd33b3

Remove Excess Fragment
b918ee2

Remove empty line
f3c38e9

Remove extra fragment (bam table)
c687f57

Reorg imports
85ec50a

Merge branch 'iobio' ofhttps://github.com/overture-stack/stageinto …
c18e72d
…feat/55-load-files

Context Consumer / SelectedRows
cf85bd7

Get Selected Bam File
b327e49

Download URL Function + Config
f92b78f

Extend error message
e59e9fb

Draft Score Download
99d17bd

Testing State
5955cbe

Add Metadata Type
0e3c995

Updated Async / Import
e738a23

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

justincorrigible reviewed Sep 24, 2024
View reviewed changes

Comment threadcomponents/pages/explorer/fileUtils.ts Outdated

Null handling + remove extra 'if'
0f6d20a

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

ciaranschutte suggested changes Sep 25, 2024
View reviewed changes

demariadaniel added 4 commits September 25, 2024 10:36

Score Url env.schema
9946bb0

Move Demo Data to its own file
fddb126

Add missing await
7cec0c6

Remove useMemo
880c9f0

Rename setElementState
7149feb

justincorrigible reviewed Sep 25, 2024
View reviewed changes

Comment thread.env.schema

demariadaniel added 12 commits September 25, 2024 11:20

Add env schema comment
10019ad

Remove Unused useRouter
12570a9

Change undefined syntax
e972f32

Change Axios Promise handling
1d96ef0

Return axios promise
282bc7d

Remove unused await
1cdce40

Prefer async for consistency
d1abf7a

Fix catch syntax
c79f08d

Remove error testing
4dd9e22

Type Narrowing
33629a4

Move Demo File function
f52fbb4

Update Loading Hook
5872309

ciaranschutte self-requested a review September 27, 2024 23:42

ciaranschutte approved these changes Sep 27, 2024
View reviewed changes

demariadaniel merged commit 484c2c0 into iobioSep 30, 2024

demariadaniel deleted the feat/55-load-files branch September 30, 2024 13:47

demariadaniel added 30 commits August 30, 2024 16:32

Buttons + first Layout
45644af

Tweak Chart size
ebbb96a

Adjust Container CSS
164200a

Update File Name
a26800b

Toggle Button CSS
41bf9ec

Toggle state
54389c0

Table Markup / CSS
c1bf667

Updated Button colors w/ Icon placeholder
2b5e07c

Style var cleanup
e042f17

Display: flex / use flex-start
25e25b4

Missing read coverage toggle
bdaa6e1

Import context from Iobio Library
f78f6eb

Add Icons
b7b1224

Reusable Functions + Variables
f340ce4

JSX rule names
42004d0

Copyright corrections
752853c

Latest iobio package
d90787d

isFileTableActive var name
ccd33b3

Remove Excess Fragment
b918ee2

Remove empty line
f3c38e9

Remove extra fragment (bam table)
c687f57

Reorg imports
85ec50a

Merge branch 'iobio' ofhttps://github.com/overture-stack/stageinto …
c18e72d
…feat/55-load-files

Context Consumer / SelectedRows
cf85bd7

Get Selected Bam File
b327e49

Download URL Function + Config
f92b78f

Extend error message
e59e9fb

Draft Score Download
99d17bd

Testing State
5955cbe

Add Metadata Type
0e3c995

Updated Async / Import
e738a23

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

justincorrigible reviewed Sep 24, 2024
View reviewed changes

Comment threadcomponents/pages/explorer/fileUtils.ts Outdated

Null handling + remove extra 'if'
0f6d20a

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

ciaranschutte suggested changes Sep 25, 2024
View reviewed changes

demariadaniel added 4 commits September 25, 2024 10:36

Score Url env.schema
9946bb0

Move Demo Data to its own file
fddb126

Add missing await
7cec0c6

Remove useMemo
880c9f0

Rename setElementState
7149feb

justincorrigible reviewed Sep 25, 2024
View reviewed changes

Comment thread.env.schema

demariadaniel added 12 commits September 25, 2024 11:20

Add env schema comment
10019ad

Remove Unused useRouter
12570a9

Change undefined syntax
e972f32

Change Axios Promise handling
1d96ef0

Return axios promise
282bc7d

Remove unused await
1cdce40

Prefer async for consistency
d1abf7a

Fix catch syntax
c79f08d

Remove error testing
4dd9e22

Type Narrowing
33629a4

Move Demo File function
f52fbb4

Update Loading Hook
5872309

ciaranschutte self-requested a review September 27, 2024 23:42

ciaranschutte approved these changes Sep 27, 2024
View reviewed changes

demariadaniel merged commit 484c2c0 into iobioSep 30, 2024

demariadaniel deleted the feat/55-load-files branch September 30, 2024 13:47

demariadaniel added 30 commits August 30, 2024 16:32

Buttons + first Layout
45644af

Tweak Chart size
ebbb96a

Adjust Container CSS
164200a

Update File Name
a26800b

Toggle Button CSS
41bf9ec

Toggle state
54389c0

Table Markup / CSS
c1bf667

Updated Button colors w/ Icon placeholder
2b5e07c

Style var cleanup
e042f17

Display: flex / use flex-start
25e25b4

Missing read coverage toggle
bdaa6e1

Import context from Iobio Library
f78f6eb

Add Icons
b7b1224

Reusable Functions + Variables
f340ce4

JSX rule names
42004d0

Copyright corrections
752853c

Latest iobio package
d90787d

isFileTableActive var name
ccd33b3

Remove Excess Fragment
b918ee2

Remove empty line
f3c38e9

Remove extra fragment (bam table)
c687f57

Reorg imports
85ec50a

Merge branch 'iobio' ofhttps://github.com/overture-stack/stageinto …
c18e72d
…feat/55-load-files

Context Consumer / SelectedRows
cf85bd7

Get Selected Bam File
b327e49

Download URL Function + Config
f92b78f

Extend error message
e59e9fb

Draft Score Download
99d17bd

Testing State
5955cbe

Add Metadata Type
0e3c995

Updated Async / Import
e738a23

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

justincorrigible reviewed Sep 24, 2024
View reviewed changes

Comment threadcomponents/pages/explorer/fileUtils.ts Outdated

Null handling + remove extra 'if'
0f6d20a

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

ciaranschutte suggested changes Sep 25, 2024
View reviewed changes

demariadaniel added 4 commits September 25, 2024 10:36

Score Url env.schema
9946bb0

Move Demo Data to its own file
fddb126

Add missing await
7cec0c6

Remove useMemo
880c9f0

Rename setElementState
7149feb

justincorrigible reviewed Sep 25, 2024
View reviewed changes

Comment thread.env.schema

demariadaniel added 12 commits September 25, 2024 11:20

Add env schema comment
10019ad

Remove Unused useRouter
12570a9

Change undefined syntax
e972f32

Change Axios Promise handling
1d96ef0

Return axios promise
282bc7d

Remove unused await
1cdce40

Prefer async for consistency
d1abf7a

Fix catch syntax
c79f08d

Remove error testing
4dd9e22

Type Narrowing
33629a4

Move Demo File function
f52fbb4

Update Loading Hook
5872309

ciaranschutte self-requested a review September 27, 2024 23:42

ciaranschutte approved these changes Sep 27, 2024
View reviewed changes

demariadaniel merged commit 484c2c0 into iobioSep 30, 2024

demariadaniel deleted the feat/55-load-files branch September 30, 2024 13:47

demariadaniel added 30 commits August 30, 2024 16:32

Buttons + first Layout
45644af

Tweak Chart size
ebbb96a

Adjust Container CSS
164200a

Update File Name
a26800b

Toggle Button CSS
41bf9ec

Toggle state
54389c0

Table Markup / CSS
c1bf667

Updated Button colors w/ Icon placeholder
2b5e07c

Style var cleanup
e042f17

Display: flex / use flex-start
25e25b4

Missing read coverage toggle
bdaa6e1

Import context from Iobio Library
f78f6eb

Add Icons
b7b1224

Reusable Functions + Variables
f340ce4

JSX rule names
42004d0

Copyright corrections
752853c

Latest iobio package
d90787d

isFileTableActive var name
ccd33b3

Remove Excess Fragment
b918ee2

Remove empty line
f3c38e9

Remove extra fragment (bam table)
c687f57

Reorg imports
85ec50a

Merge branch 'iobio' ofhttps://github.com/overture-stack/stageinto …
c18e72d
…feat/55-load-files

Context Consumer / SelectedRows
cf85bd7

Get Selected Bam File
b327e49

Download URL Function + Config
f92b78f

Extend error message
e59e9fb

Draft Score Download
99d17bd

Testing State
5955cbe

Add Metadata Type
0e3c995

Updated Async / Import
e738a23

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

justincorrigible reviewed Sep 24, 2024
View reviewed changes

Comment threadcomponents/pages/explorer/fileUtils.ts Outdated

Null handling + remove extra 'if'
0f6d20a

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

ciaranschutte suggested changes Sep 25, 2024
View reviewed changes

demariadaniel added 4 commits September 25, 2024 10:36

Score Url env.schema
9946bb0

Move Demo Data to its own file
fddb126

Add missing await
7cec0c6

Remove useMemo
880c9f0

Rename setElementState
7149feb

justincorrigible reviewed Sep 25, 2024
View reviewed changes

Comment thread.env.schema

demariadaniel added 12 commits September 25, 2024 11:20

Add env schema comment
10019ad

Remove Unused useRouter
12570a9

Change undefined syntax
e972f32

Change Axios Promise handling
1d96ef0

Return axios promise
282bc7d

Remove unused await
1cdce40

Prefer async for consistency
d1abf7a

Fix catch syntax
c79f08d

Remove error testing
4dd9e22

Type Narrowing
33629a4

Move Demo File function
f52fbb4

Update Loading Hook
5872309

ciaranschutte self-requested a review September 27, 2024 23:42

ciaranschutte approved these changes Sep 27, 2024
View reviewed changes

demariadaniel merged commit 484c2c0 into iobioSep 30, 2024

demariadaniel deleted the feat/55-load-files branch September 30, 2024 13:47

demariadaniel added 30 commits August 30, 2024 16:32

Buttons + first Layout
45644af

Tweak Chart size
ebbb96a

Adjust Container CSS
164200a

Update File Name
a26800b

Toggle Button CSS
41bf9ec

Toggle state
54389c0

Table Markup / CSS
c1bf667

Updated Button colors w/ Icon placeholder
2b5e07c

Style var cleanup
e042f17

Display: flex / use flex-start
25e25b4

Missing read coverage toggle
bdaa6e1

Import context from Iobio Library
f78f6eb

Add Icons
b7b1224

Reusable Functions + Variables
f340ce4

JSX rule names
42004d0

Copyright corrections
752853c

Latest iobio package
d90787d

isFileTableActive var name
ccd33b3

Remove Excess Fragment
b918ee2

Remove empty line
f3c38e9

Remove extra fragment (bam table)
c687f57

Reorg imports
85ec50a

Merge branch 'iobio' ofhttps://github.com/overture-stack/stageinto …
c18e72d
…feat/55-load-files

Context Consumer / SelectedRows
cf85bd7

Get Selected Bam File
b327e49

Download URL Function + Config
f92b78f

Extend error message
e59e9fb

Draft Score Download
99d17bd

Testing State
5955cbe

Add Metadata Type
0e3c995

Updated Async / Import
e738a23

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

justincorrigible reviewed Sep 24, 2024
View reviewed changes

Comment threadcomponents/pages/explorer/fileUtils.ts Outdated

Null handling + remove extra 'if'
0f6d20a

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

ciaranschutte suggested changes Sep 25, 2024
View reviewed changes

demariadaniel added 4 commits September 25, 2024 10:36

Score Url env.schema
9946bb0

Move Demo Data to its own file
fddb126

Add missing await
7cec0c6

Remove useMemo
880c9f0

Rename setElementState
7149feb

justincorrigible reviewed Sep 25, 2024
View reviewed changes

Comment thread.env.schema

demariadaniel added 12 commits September 25, 2024 11:20

Add env schema comment
10019ad

Remove Unused useRouter
12570a9

Change undefined syntax
e972f32

Change Axios Promise handling
1d96ef0

Return axios promise
282bc7d

Remove unused await
1cdce40

Prefer async for consistency
d1abf7a

Fix catch syntax
c79f08d

Remove error testing
4dd9e22

Type Narrowing
33629a4

Move Demo File function
f52fbb4

Update Loading Hook
5872309

ciaranschutte self-requested a review September 27, 2024 23:42

ciaranschutte approved these changes Sep 27, 2024
View reviewed changes

demariadaniel merged commit 484c2c0 into iobioSep 30, 2024

demariadaniel deleted the feat/55-load-files branch September 30, 2024 13:47

demariadaniel added 30 commits August 30, 2024 16:32

Buttons + first Layout
45644af

Tweak Chart size
ebbb96a

Adjust Container CSS
164200a

Update File Name
a26800b

Toggle Button CSS
41bf9ec

Toggle state
54389c0

Table Markup / CSS
c1bf667

Updated Button colors w/ Icon placeholder
2b5e07c

Style var cleanup
e042f17

Display: flex / use flex-start
25e25b4

Missing read coverage toggle
bdaa6e1

Import context from Iobio Library
f78f6eb

Add Icons
b7b1224

Reusable Functions + Variables
f340ce4

JSX rule names
42004d0

Copyright corrections
752853c

Latest iobio package
d90787d

isFileTableActive var name
ccd33b3

Remove Excess Fragment
b918ee2

Remove empty line
f3c38e9

Remove extra fragment (bam table)
c687f57

Reorg imports
85ec50a

Merge branch 'iobio' ofhttps://github.com/overture-stack/stageinto …
c18e72d
…feat/55-load-files

Context Consumer / SelectedRows
cf85bd7

Get Selected Bam File
b327e49

Download URL Function + Config
f92b78f

Extend error message
e59e9fb

Draft Score Download
99d17bd

Testing State
5955cbe

Add Metadata Type
0e3c995

Updated Async / Import
e738a23

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

justincorrigible reviewed Sep 24, 2024
View reviewed changes

Comment threadcomponents/pages/explorer/fileUtils.ts Outdated

Null handling + remove extra 'if'
0f6d20a

ciaranschutte reviewed Sep 24, 2024
View reviewed changes

ciaranschutte suggested changes Sep 25, 2024
View reviewed changes

demariadaniel added 4 commits September 25, 2024 10:36

Score Url env.schema
9946bb0

Move Demo Data to its own file
fddb126

Add missing await
7cec0c6

Remove useMemo
880c9f0

Rename setElementState
7149feb

justincorrigible reviewed Sep 25, 2024
View reviewed changes

Comment thread.env.schema

demariadaniel added 12 commits September 25, 2024 11:20

Add env schema comment
10019ad

Remove Unused useRouter
12570a9

Change undefined syntax
e972f32

Change Axios Promise handling
1d96ef0

Return axios promise
282bc7d

Remove unused await
1cdce40

Prefer async for consistency
d1abf7a

Fix catch syntax
c79f08d

Remove error testing
4dd9e22

Type Narrowing
33629a4

Move Demo File function
f52fbb4

Update Loading Hook
5872309

ciaranschutte self-requested a review September 27, 2024 23:42

ciaranschutte approved these changes Sep 27, 2024
View reviewed changes

demariadaniel merged commit 484c2c0 into iobioSep 30, 2024

demariadaniel deleted the feat/55-load-files branch September 30, 2024 13:47

Uh oh!

Conversation

demariadaniel commented Sep 10, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

demariadanielSep 24, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

demariadanielSep 25, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ciaranschutte left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

demariadanielSep 25, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

demariadaniel commented Sep 25, 2024

Uh oh!

Uh oh!

ciaranschutte left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

demariadaniel commented Sep 10, 2024•
edited
Loading

demariadanielSep 24, 2024•
edited
Loading

demariadanielSep 25, 2024•
edited
Loading

demariadanielSep 25, 2024•
edited
Loading

demariadaniel commented Sep 10, 2024•
edited
Loading

demariadanielSep 24, 2024•
edited
Loading