Uploaded image for project: 'IGB'
  1. IGB
  2. IGBF-4098

Fix Color by for Samtools to have persistent user input

    Details

    • Type: Task
    • Status: Closed (View Workflow)
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: 10.2.0
    • Labels:
      None

      Description

      Currently, the Samtools tags option for Color by has a table that needs to be filled by the user. However, the table data disappears after the user clicks OK.

      Fix: The table data needs to be persistent through out an IGB session.

        Attachments

          Issue Links

            Activity

            Hide
            karthik Karthik Raveendran added a comment -
            Show
            karthik Karthik Raveendran added a comment - See Commit
            Hide
            nfreese Nowlan Freese added a comment - - edited

            Tested Karthik's branch on Mac.

            Tags and their respective colors persist after closing and reopening the Color by window, no errors in the log. The one issue I see is if the user provides multiple tag values in a single input row, these are then split into separate rows for each tag value. I think they should remain in a single row.

            Show
            nfreese Nowlan Freese added a comment - - edited Tested Karthik's branch on Mac. Tags and their respective colors persist after closing and reopening the Color by window, no errors in the log. The one issue I see is if the user provides multiple tag values in a single input row, these are then split into separate rows for each tag value. I think they should remain in a single row.
            Hide
            karthik Karthik Raveendran added a comment - - edited

            Fixed to retain user input format and avoid splitting the input. See commit

            Show
            karthik Karthik Raveendran added a comment - - edited Fixed to retain user input format and avoid splitting the input. See commit
            Show
            karthik Karthik Raveendran added a comment - PR Submitted https://bitbucket.org/lorainelab/integrated-genome-browser/pull-requests/1068
            Hide
            ann.loraine Ann Loraine added a comment - - edited

            PR is merged. Installers are built and available from the download site on bitbucket but not yet deployed to the early access section of bioviz.org due to a problem with space on the EC2 host.

            Working on it.

            In the meantime, please test the installers available on the download site of bitbucket.

            Ready for testing.

            Show
            ann.loraine Ann Loraine added a comment - - edited PR is merged. Installers are built and available from the download site on bitbucket but not yet deployed to the early access section of bioviz.org due to a problem with space on the EC2 host. Working on it. In the meantime, please test the installers available on the download site of bitbucket. Ready for testing.
            Hide
            pkulzer Paige Kulzer (Inactive) added a comment -

            I downloaded the newest installer from BitBucket and tested on my Mac using the single cell quickload that Karthik made. Here are some things I noticed during testing, many of which might be edge cases that can be followed up on in subsequent tickets:

            1. The Color by table is not maintaining any edits after having been reopened once. For example, adding "acat" to the table, then closing and reopening, the input persists. However, editing that same value to "acatacat", then closing and reopening, the input reverts back to "acat".
            2. The Color by table reorders itself after closing and reopening the window for the first time. I'm not sure what it's reordering by since it doesn't appear to be the Tag Value column, but I think simply shifting input up if a user left some blank spaces in the table would make more sense than reordering.
            3. I opened the Color by table, left it empty, clicked OK and Apply, then IGB threw the following error and broke. The track was removed from the screen and trying to toggle it back via the Data Management Table doesn't work, so IGB has to be restarted.
              Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException: Cannot invoke "java.util.HashMap.forEach(java.util.function.BiConsumer)" because "input_table_values" is null
              
            4. I opened the Color by table and added some values as usual. Then, I reopened only the Color by window, so not opening up the table, then clicking OK. This throws an error and IGB behaves just like it did with the above scenario.
              Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException: Cannot invoke "java.util.HashMap.forEach(java.util.function.BiConsumer)" because "input_table_values" is null
              
            5. When I set a tag in the Color by table without a color assigned, then close and reopen the table, that entry is deleted. Is this expected behavior? Or should that value persist?
            6. The tag being set in the Color by window is not being maintained between closing and reopening of that window, but I think making this input persistent as well would make more sense for a user.

            Karthik Raveendran, please review my comments above and let me know if you have any questions!

            Show
            pkulzer Paige Kulzer (Inactive) added a comment - I downloaded the newest installer from BitBucket and tested on my Mac using the single cell quickload that Karthik made. Here are some things I noticed during testing, many of which might be edge cases that can be followed up on in subsequent tickets: The Color by table is not maintaining any edits after having been reopened once. For example, adding "acat" to the table, then closing and reopening, the input persists. However, editing that same value to "acatacat", then closing and reopening, the input reverts back to "acat". The Color by table reorders itself after closing and reopening the window for the first time. I'm not sure what it's reordering by since it doesn't appear to be the Tag Value column, but I think simply shifting input up if a user left some blank spaces in the table would make more sense than reordering. I opened the Color by table, left it empty, clicked OK and Apply, then IGB threw the following error and broke. The track was removed from the screen and trying to toggle it back via the Data Management Table doesn't work, so IGB has to be restarted. Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException: Cannot invoke "java.util.HashMap.forEach(java.util.function.BiConsumer)" because "input_table_values" is null I opened the Color by table and added some values as usual. Then, I reopened only the Color by window , so not opening up the table, then clicking OK. This throws an error and IGB behaves just like it did with the above scenario. Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException: Cannot invoke "java.util.HashMap.forEach(java.util.function.BiConsumer)" because "input_table_values" is null When I set a tag in the Color by table without a color assigned, then close and reopen the table, that entry is deleted. Is this expected behavior? Or should that value persist? The tag being set in the Color by window is not being maintained between closing and reopening of that window, but I think making this input persistent as well would make more sense for a user. Karthik Raveendran , please review my comments above and let me know if you have any questions!
            Hide
            karthik Karthik Raveendran added a comment - - edited

            1) When adding a value (or editing it) to the Tag Value column, the user has to either press enter or click away for that value to be associated to that table cell. The value is then read from the cell and saved, when Save and Apply is clicked. This is default behavior of the table and will need fixing because a user is expected to behave as Paige Kulzer did. Needs to be fixed.

            2) Table reorders itself because the HashMap is being used which does not preserve the order. May not be fixed in this ticket
            3) & 4) Null condition needs to be checked here. Needs to be fixed.
            5) As of right now, the user needs to add a color value to save the data. If color value is not added the tag value is not of concern to a Color by function. However, we can default it to light gray as well. Not sure if this needs fixing
            6) This is an issue that's existed in the Color by options. May need to be fixed in the future

            Show
            karthik Karthik Raveendran added a comment - - edited 1) When adding a value (or editing it) to the Tag Value column, the user has to either press enter or click away for that value to be associated to that table cell. The value is then read from the cell and saved, when Save and Apply is clicked. This is default behavior of the table and will need fixing because a user is expected to behave as Paige Kulzer did. Needs to be fixed. 2) Table reorders itself because the HashMap is being used which does not preserve the order. May not be fixed in this ticket 3) & 4) Null condition needs to be checked here. Needs to be fixed. 5) As of right now, the user needs to add a color value to save the data. If color value is not added the tag value is not of concern to a Color by function. However, we can default it to light gray as well. Not sure if this needs fixing 6) This is an issue that's existed in the Color by options. May need to be fixed in the future
            Hide
            karthik Karthik Raveendran added a comment -

            Issues mention above are fixed. See commit

            Show
            karthik Karthik Raveendran added a comment - Issues mention above are fixed. See commit
            Hide
            pkulzer Paige Kulzer (Inactive) added a comment -

            I forked Karthik's branch and tested the Color by feature. Issues #1, 3, and 4 all appear to be fixed with this new commit. The Color by table is now retaining user input after each closing and reopening of the table, and no errors are being thrown when the Color by window or table are closed.

            I found one other small feature that I wanted to mention of this ticket (again, this is likely out of scope): The values in the Color by table are persisting even after switching genomes. A Clear All or Delete All button might be a good compromise for this if there's no way to detect when a genome's been changed. That would provide user's with a quick way to reset the table.

            Recommending PR!

            Show
            pkulzer Paige Kulzer (Inactive) added a comment - I forked Karthik's branch and tested the Color by feature. Issues #1, 3, and 4 all appear to be fixed with this new commit. The Color by table is now retaining user input after each closing and reopening of the table, and no errors are being thrown when the Color by window or table are closed. I found one other small feature that I wanted to mention of this ticket (again, this is likely out of scope): The values in the Color by table are persisting even after switching genomes. A Clear All or Delete All button might be a good compromise for this if there's no way to detect when a genome's been changed. That would provide user's with a quick way to reset the table. Recommending PR!
            Hide
            karthik Karthik Raveendran added a comment - - edited

            PR Submitted https://bitbucket.org/lorainelab/integrated-genome-browser/pull-requests/1073

            Note: The clear all button issue mentioned by Paige Kulzer above will be addressed in IGBF-4195

            Show
            karthik Karthik Raveendran added a comment - - edited PR Submitted https://bitbucket.org/lorainelab/integrated-genome-browser/pull-requests/1073 Note: The clear all button issue mentioned by Paige Kulzer above will be addressed in IGBF-4195
            Hide
            ann.loraine Ann Loraine added a comment - - edited

            PR is merged. Installers are built and deployed to Bioviz.org Early Access section.

            Ready for testing.

            Show
            ann.loraine Ann Loraine added a comment - - edited PR is merged. Installers are built and deployed to Bioviz.org Early Access section. Ready for testing.
            Hide
            pkulzer Paige Kulzer (Inactive) added a comment -

            Tested on Mac, entries in the Color by table are now persisting between the closing and re-opening of the table. This is true whether a user presses enter/clicks away after entering a Tag Value or not. No errors are appearing in the log.

            Closing this ticket.

            Show
            pkulzer Paige Kulzer (Inactive) added a comment - Tested on Mac, entries in the Color by table are now persisting between the closing and re-opening of the table. This is true whether a user presses enter/clicks away after entering a Tag Value or not. No errors are appearing in the log. Closing this ticket.

              People

              • Assignee:
                karthik Karthik Raveendran
                Reporter:
                karthik Karthik Raveendran
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: