GRNsight icon indicating copy to clipboard operation
GRNsight copied to clipboard

Additional clean-up of export to Excel function/interface

Open kdahlquist opened this issue 1 year ago • 35 comments

I did the following actions:

  • launch GRNsight beta 7.2.2
  • Load a GRN from database
  • Export to Excel
  • This is what the modal looks like:

Image

  • Under the top heading of "Select the Expression Data Source:", GRN (Yeastmine - SGD: 2024-03-19; 8 genes, 8 edges) should not be one of the options. That is specifying the source of the GRN, not the source of the expression data.
    • It makes sense to have a button there when a user uploads a file with expression data, but not when a user loads from the GRN database because the only expression data would be coming from the expression database.
  • Also, the suggested filename was: "GRN (Yeastmine - SGD_ 2024-03-19; 8 genes, 8 edges)_weighted.xlsx". However, this export was unweighted.

Image

  • Note in the screenshot above, when the filename is really long the spacing of the button and filename is messed up.

kdahlquist avatar Feb 19 '25 16:02 kdahlquist

I found the code that generates the source as one of the options, lines 392-395, but I haven't figured out how to take out those lines without creating other errors yet. On line 393, value=\"userInput\" takes in and shows whatever the user selected, but it doesn't differentiate between files that are user-uploaded or what's from the GRNsight database so perhaps that needs to be checked first. Lines 392-395 create the first li option (with the User-Input as the value) and then the other options are added on. It seems like if lines 392-395 are taken out, then the options are not nested within the same ul and errors occur, preventing the generation of the options and pop up.

Image

Amelie1253 avatar Apr 02 '25 07:04 Amelie1253

It looks like the approach to take here will need to be more similar to the loop in line 397: that loop iterates through the data sources and appends an li string to the result variable for each one

Similarly, the li item in line 392 may need to be appended dynamically as well. The result variable should be initialized without that li item, and then you may need to move that to a string append based on a condition, something like “if the current network came from a file with its own expression data, then add that item”

dondi avatar Apr 02 '25 15:04 dondi

Upon trying out the feature during the 4/2 meeting, @dondi pointed out that it would also be nice to pre-select the currently-displayed expression data for export, since the user will be looking at that data during this time, their mental model is likely inclined to think that when they choose “export,” they will want to include the expression data set that they’re currently seeing

dondi avatar Apr 02 '25 15:04 dondi

I think I resolved the primary issue of removing the source of the GRN as one of the options for Expression Data Source. I am a little confused on the conditional. Could you clarify the "if the current network came from a file with its own expression data, then add that item"?

Amelie1253 avatar Apr 30 '25 15:04 Amelie1253

Could you clarify the "if the current network came from a file with its own expression data, then add that item"?

Users can load a GRN by selecting "Network" > "Open File" and selecting an Excel workbook. One (or more) of the worksheets in the workbook can contain expression data that will then display on the nodes of the GRN graph. These data come from the workbook that the user uploaded and not from our internal expression database. Does this help?

kdahlquist avatar Aug 26 '25 21:08 kdahlquist

Yes that helps! @Amelie1253 and I reviewed our solution from the spring and just wanted to check that the functionality is the same as intended. The export no longer shows the source of the GRN as one of the Expression Data Source options when demos are being exported, but we want the name of the user-uploaded file to be the pre-selected option when the user uploads an Excel notebook, right?

MilkaZek avatar Sep 03 '25 17:09 MilkaZek

Yes that helps! @Amelie1253 and I reviewed our solution from the spring and just wanted to check that the functionality is the same as intended. The export no longer shows the source of the GRN as one of the Expression Data Source options when demos are being exported, but we want the name of the user-uploaded file to be the pre-selected option when the user uploads an Excel notebook, right?

Yes, the user-uploaded file should be the pre-selected option when the user uploads an Excel workbook (if it has expression data in it). If the user-uploaded workbook doesn't have expression data, that opt-group would be empty. The graph should default to node coloring "off".

kdahlquist avatar Sep 03 '25 17:09 kdahlquist

While you are working on this, there needs to be an option to not export any Expression Data. Since the dataset options are radio buttons, the user is forced to select one. "None" should be an option on the list. It can be at the top or the bottom of the list, whichever seems better.

kdahlquist avatar Sep 05 '25 21:09 kdahlquist

I have updated our solution to add the None option by just append another <li> option for None. I think that this solution aligns with what was intended but would like to revisit it and check that the functionality works as intended.

Amelie1253 avatar Sep 10 '25 06:09 Amelie1253

I have updated our solution to add the None option by just append another <li> option for None. I think that this solution aligns with what was intended but would like to revisit it and check that the functionality works as intended.

Note that if an expression dataset is selected, but then the user unchecks the box(es) lower down to deselect all the expression sheets, then the radio button should then move to "None".

kdahlquist avatar Sep 10 '25 21:09 kdahlquist

Apologies, this week has been busy for me and I wasn't able to work on this issue much! For clarification on @kdahlquist's comment, if the user deselects the Select All checkbox under "Select Workbook Sheets to Export" or if the user deselects all of the sheets manually, the Expression Data Source radio button should move to None automatically?

Amelie1253 avatar Sep 17 '25 15:09 Amelie1253

For clarification on @kdahlquist's comment, if the user deselects the Select All checkbox under "Select Workbook Sheets to Export" or if the user deselects all of the sheets manually, the Expression Data Source radio button should move to None automatically?

Correct.

kdahlquist avatar Sep 17 '25 16:09 kdahlquist

I think I misunderstood what functionality was desired and the fixes I had earlier only partially address the issue. My solution simply added the option for expression data when it was user uploaded which is semi-correct but it doesn't check where expression data is coming from, which is the root of the issue. So I'll be reworking my solution to check for the expression data as well as incorporating what to do when None is selected. I'm also working on understanding how the export works as I am a little confused.

Amelie1253 avatar Sep 24 '25 17:09 Amelie1253

This was discussed further at the meeting and the hope is that the clarifications will allow this to move forward; the core principle is to establish better continuity between the network being displayed (particularly whether that network had expression data on its own, which would only happen if the network is either a demo or loaded from a workbook that had its own expression data tabs), the currently selected data set, and whether the user would like to export expression data at all

dondi avatar Sep 24 '25 17:09 dondi

Breaking issue down into sub-issues and where I'm at:

  • There should be some way to check for expression data that does not come from our database. This expression data will either be from a Demo or from a user uploaded file and should appear as an option.
    • the function that create these options loops through the data to append those options for expression data.
    • currently I use grnState.workbook.expressionNames to check if there is expression data before appending an option
      • this resolves the issue of the GRN appearing as an option.
  • The option for the currently-displayed expression data for export should be pre-selected as UX design choice/style preference.
    • I haven't dug into this functionality yet.
  • Otherwise (if there is no expression data), have a None option.
    • I've been able to add None to the list of options but have to figure out how to handle the option of None when exporting.
    • I've been looking at the handleExportExcelModal as that is where the actual exporting happens but don't fully understand it yet. It also seems like exporting no expression data might touch some other functions as well.

This was kind of a recap for me and to also confirm that I am working in the right direction. For next week, I will target getting the functionality of exporting no expression working.

Amelie1253 avatar Oct 01 '25 05:10 Amelie1253

Thank you for your detailed notes. This is a model for what comments on issues should look like!

kdahlquist avatar Oct 01 '25 18:10 kdahlquist

For the None functionality, I found the handleExpressionDataAndExport function. This function handles the Demo, uploaded files, and GRN nodes, and formats the export according to the type. If there is no expression data, this function should skip adding expression data. I am working on a way to check for expression data from the upload and then properly format the export without the expression data.

Amelie1253 avatar Oct 15 '25 16:10 Amelie1253

Expression data is likely an attribute in whichever variable holds the overall “network” object (i.e., the data structure representing the overall GRN/PPI). Once the name of that attribute is identified, building logic around the presence/absence of that attribute should be straightforward

dondi avatar Oct 15 '25 17:10 dondi

A quick inspection of the JSON returned by our API showed that the attribute in question is most likely expression, and instead of presence or absence, code will need to check whether the object is empty vs. not

dondi avatar Oct 15 '25 17:10 dondi

Sorry if this is a silly question but how do I test uploading files without expression data? Is there a way I can get files without expression data?

Since I'm working on getting that functionality to work, I'm not sure if I can create my own files without expression data through exporting?

Amelie1253 avatar Oct 17 '25 21:10 Amelie1253

My last comment was a bit hasty. I think I got the None functionality working! I did add a check for if the expression attribute in the JSON is empty and then exports without expression data.

After testing a bit more, I'll move onto working on moving the radio button to None if all expression sheets are deselected and then pre-selecting the currently-displayed expression data option.

Amelie1253 avatar Oct 20 '25 05:10 Amelie1253

Sorry if this is a silly question but how do I test uploading files without expression data? Is there a way I can get files without expression data?

Since I'm working on getting that functionality to work, I'm not sure if I can create my own files without expression data through exporting?

Sorry for not responding sooner. Bear in mind that you can edit the Excel files in Excel to create whatever test files that you need. There are several test cases for missing expression data.

  1. Where the sheet is missing entirely
  2. Where the sheet exists, but is blank
  3. The sheet exists and has IDs and headers, but there is no data

kdahlquist avatar Oct 21 '25 21:10 kdahlquist

Got it! That helped me better test the functionality of None. As I looked into the specific files themselves and compared a file with expression data and one without I found that:

While my solution does export a file without the expression data, it also deletes the optimization parameters sheet. I am unsure of how those are related. I was able to upload this file and display a network graph without any coloring.

I also tested for uploading files without an expression sheet, a blank expression sheet, and an expression with only IDs and headers. All three generated errors.

I'm unsure of why the optimization parameters seem to cause such errors or be related to causing errors?

Amelie1253 avatar Oct 22 '25 16:10 Amelie1253

When GRNsight was first developed, the only sheets that could be uploaded was "network" and "network_optimzed_weights" for uncolored and colored edges, respectively. We then added the ability to read in all of the other worksheets from the GRNmap modeling software, including the expression sheets. However, we did not fully test the functionality of this new import in the same way that we tested the original "network" and "network_optimized_weights" sheet. I'm not surprised that there are errors/bugs. I'm multitasking right now and will post additional comments later.

kdahlquist avatar Oct 22 '25 17:10 kdahlquist

@dondi confirmed that this may well just be lack of thoroughness in trying all of the use cases; @Amelie1253 confirmed that the errors are console-level errors meaning that they are uncaught conditions within the existing code base

This is an opportunity to expand our unit test coverage so that once fixed, this behavior can be continually tested from now on

dondi avatar Oct 22 '25 17:10 dondi

@dondi walked @Amelie1253 and @MilkaZek through the testing code base and talked briefly about TDD (writing tests first) so we’ll see if we can go through this process firsthand with this ticket

dondi avatar Oct 22 '25 18:10 dondi

There is a separate issue for testing the import/export interface #1084, from which I generated multiple separate issues. I will post something on that issue instead because it more directly addresses testing.

kdahlquist avatar Oct 22 '25 19:10 kdahlquist

After discussing with @dondi, I am focusing on wrapping up the UI functionality of this issue. To recap for myself:

  • I have the None option and export working. This ensures that if there is expression data that does not originate from our database, it shows up as an option and there is an option for None if the user does not want to export any expression data or if there is no expression data. This None option appears and is able to export a file without expression data.

    • I'm currently checking if the optimization parameters sheet gets deleted as well or if it is exported under a different form/name.
  • WIP: pre-selecting the currently-displayed expression data and making sure that the file exports with the correct weighted or unweighted naming.

Amelie1253 avatar Oct 25 '25 01:10 Amelie1253

I think we may have discussed this previously but wanted to check. From my understanding, if a demo is displayed, there should not be an option for expression data source from the demo. There is only a separate expression data source option for an uploaded file, correct?

Amelie1253 avatar Oct 25 '25 02:10 Amelie1253

I think we may have discussed this previously but wanted to check. From my understanding, if a demo is displayed, there should not be an option for expression data source from the demo. There is only a separate expression data source option for an uploaded file, correct?

This is a little tricky to understand. Demos 1-4 do have their own expression data, which is actually a duplicate of the Dahlquist_2018 data in the expression database. The current behavior is that these are treated as two different expression data sources. If you load a demo and Export to Excel, you can see that if you select the demo as source, the names of the expression worksheets are different than if you select Dahlquist_2018 as the source. I think we should go ahead and keep this behavior.

kdahlquist avatar Oct 28 '25 17:10 kdahlquist