Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add metagenomics long read option and logic to UI #1191

Merged
merged 6 commits into from
Apr 5, 2024

Conversation

pkalita-lbl
Copy link
Collaborator

@pkalita-lbl pkalita-lbl commented Mar 18, 2024

Fixes microbiomedata/submission-schema#168

These changes add a new "Metagenome (Long Read)" checkbox to the Multiomics Data screen in the submission portal. If selected, it enables a new corresponding tab in the DataHarmonizer screen. It all works analogously to the existing checkboxes and tabs.

It turned out that once that was implemented, switching between the Metagenome tab and the Metagenome (Long Read) tab surfaced a bug in the version of Handsontable used by [email protected] related to column headers not being updated correctly. This bug was fixed in the version of Handsontable used by the latest version of DataHarmonizer.

Upgrading from DataHarmonizer 1.4.7 to 1.6.5 actually corresponds to quite a large jump in Handsontable (7.4.2 to 13.1.0). Despite that, things seems to generally work fine. The one issue I found was that in certain situations columns were not sized correctly based on their header content. This was resolved by:

  1. Moving the import of DataHarmoinzer's CSS (which includes Handsontable's CSS) up to the root level
  2. Changing the order of loading data and updating column settings when changing tabs.

Update!

Further testing of this branch revealed two issues which have been fixed.

  1. Validation was not working because DataHarmonizer.validate() became an async function and the call to it in HarmonizerApi.validate() needed to be awaited. Debugging this also revealed an extraneous watcher on activeInvalidCells that was unnecessary.
  2. Some interactions with the DataHarmonizer grid caused the afterSelection hook callback to throw an error due to passing in a col value of -1. I'm not 100% sure if this was a new bug caused by the DataHarmonizer upgrade or an existing one we just didn't notice before. Either way the fix was trivial.

@pkalita-lbl pkalita-lbl changed the title Issue submission schema 168 metag long read UI Add metagenomics long read option and logic to UI Mar 18, 2024
@pkalita-lbl pkalita-lbl requested a review from naglepuff March 19, 2024 15:45
@pkalita-lbl pkalita-lbl marked this pull request as ready for review March 19, 2024 15:46
@pkalita-lbl pkalita-lbl marked this pull request as draft March 19, 2024 16:03
@pkalita-lbl
Copy link
Collaborator Author

Moving this back into draft. Discussed with @naglepuff and @mslarae13 and decided to not try and sneak this in before the 2023.3 release feature freeze. So we'll just let this PR chill until after that release has been made.

@pkalita-lbl
Copy link
Collaborator Author

I think there may be some issues with validation caused by the DH upgrade. This is just a note to my future self to investigate that before marking this ready-to-review again.

@pkalita-lbl pkalita-lbl marked this pull request as ready for review March 27, 2024 20:15
@pkalita-lbl
Copy link
Collaborator Author

@naglepuff Now that Release 2024.3 is out and I've tested this branch a little more locally, I think this is ready for review and merge.

@mslarae13
Copy link
Contributor

@ssarrafan @naglepuff said he could review on Monday. Can we add to this sprint?

@ssarrafan
Copy link

@ssarrafan @naglepuff said he could review on Monday. Can we add to this sprint?

@naglepuff are you still planning to review this this week?

@naglepuff
Copy link
Collaborator

@ssarrafan Yes, I'm working on this now. I should be able to get this back to @pkalita-lbl by EOD, sorry for the delay

@pkalita-lbl pkalita-lbl requested a review from naglepuff April 5, 2024 15:40
@pkalita-lbl pkalita-lbl merged commit 8092b6c into main Apr 5, 2024
2 checks passed
@pkalita-lbl pkalita-lbl deleted the issue-submission-schema-168-metag-long-read-ui branch April 5, 2024 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Add "JGI - metagenomics - long read" class to submission schema
4 participants