This was mainly to remove the mzXML while retaining other related improvements made in the previous PRs, but I also, in the process, worked toward both:
Retaining code that I think will be used again, (once the LCMS sheet(s) have a loader), and...
Experimenting with an error strategy (a minor amount of effort) that I believe will be much easier for the user to consume (in the form of errors attached to spreadsheet cells via comments). It was necessary for me to be able to structure the sample sheet excel file creation code in a scalable way. I needed to see my idea in practice to understand it better, and to plan out the structure of the code in that next step. These changes may be removed. I only did a small portion relevant to the lcms code modifications to extract mzXMLs as a sort of exploration of the design idea on what may end up being code that gets pulled. I commented out calls to that code in form_valid, since its usage was removed as a part of the change in plans, and added comments to remove the code if it doesn't end up being used.
Affected Issues/Pull Requests
Partially addresses #829
Merges into PR #922
Next PR: #925
Supplants #888
It turns out that 888 doesn't achieve the goal of reducing the work associated with associating mzXML files. Note the following comment on slack about this:
Just as a preface, I think not including mzXMLs in the intermediate solution is copacetic with our overall goal of lowering the hurdles to submission. And while specifying a directory could be easier in some cases, we know that not everyone organizes their data in the same way, so it wouldn't make it easier in every case. We also know that the only time we need the user to go to the effort of mapping mzXMLs to samples in specific accucor files is when there are multiple scans of the same sample - but if the file names are identical in those cases, this interface doesn't solve that (since it can't include the directory).
In fact, I don't think there's a whole lot we can do for the user in the submission compilation process if there are multiple identically named files to be divided among multiple accucors, other than point out that there's no name match. After submission, we may be able to match things up by looking inside the files.
OK, so then I propose the following:
Remove the mzXML drag and drop feature (which I think we agree on)
Output an excel file using the current animal/sample template, with samples filled in
I think it's worthwhile to also allow an animal/sample sheet input (so that samples can be added). I still think that just rebranding the validation page, limiting it to 1 of each file (animal/sample and peak annotation), and removing errors related to this use case (no animal/sample sheet) is the way to go, but if I can't convince you of that, I'll skip this and make a separate page if you insist. I'm just not convinced that that goes in the right direction.
Make the peak annotation input a single field.
The other alternative I'd proposed earlier, of getting the LCMS loader done first, is moot if we're using the old template.
I did spend a few minutes BTW just now, polishing off the commit that I was 98% toward yesterday. I'll post a PR, even though some of the decisions above revert aspects of it. I'm also having second thoughts about my recommendation of limiting the interface to 1 of each (of the 2) file type(s), at least in the short term.
There's still the problem of timeouts if we try to validate the data and look for errors, but I realized that, if not given a sample sheet, we could simply not try to look for errors at all, and that would make it fast.
And while I was looking at the code just now, I realized that the reason I got on board with 1 at a time strategy was because of the identically named mzXML issue. By removing that as an obstacle, I don't see any reason we can't take a series of accucor/isocorr and output a list of all the samples in a single sample sheet.
I haven't fully fleshed this idea out, but it's enough to make me want to propose that we at least postpone implementing the limit in the next few commits/PRs. I say, that can be another incremental step, after the next meeting, and after having worked with the code.
Review Notes
See comments in-line.
Checklist
This pull request will be merged once the following requirements are met. The
author and/or reviewers should uncheck any unmet requirements:
Review requirements
Minimum approvals: 1
No changes requested
All blocking issues resolved by reviewers
Specific reviewers: @add_username_here
Review period: 2 days
Associated issue/pull request requirements:
[x] All requirements in affected issues marked "resolved" are satisfied
[x] All required pull requests are merged (or none)
Summary Change Description
This was mainly to remove the mzXML while retaining other related improvements made in the previous PRs, but I also, in the process, worked toward both:
form_valid
, since its usage was removed as a part of the change in plans, and added comments to remove the code if it doesn't end up being used.Affected Issues/Pull Requests
Supplants #888
It turns out that 888 doesn't achieve the goal of reducing the work associated with associating mzXML files. Note the following comment on slack about this:
Review Notes
See comments in-line.
Checklist
This pull request will be merged once the following requirements are met. The author and/or reviewers should uncheck any unmet requirements:
changelog.md
(or no change)