feature/combine pd and 2de - super awesome PR #40
Conversation
ShreyesGadwalkar
left a comment
ShreyesGadwalkar
left a comment
There was a problem hiding this comment.
env.ts changes - need opinion on this.
ShreyesGadwalkar
assigned
ShreyesGadwalkar,
atparsec,
jacob-fay,
ZythiQ,
ldk7811,
SabrePenguin and
VarunSB14
ZythiQ
left a comment
ZythiQ
left a comment
There was a problem hiding this comment.
LGTM - Seems like Luke's requested changes were made - thanks for catching that.
ZythiQ
dismissed
ldk7811’s stale review
The changes were reverted - LGTM and we're short on time.
|
This still regresses the peptide retention, I cannot approve it. |
…raig/ProSep into feature/combine-pd-and-2de
…raig/ProSep into feature/combine-pd-and-2de pulling
ldk7811
left a comment
ldk7811
left a comment
There was a problem hiding this comment.
I added back in basic zoom in and out for this PR, but this is not complete.
I'm gonna copy and paste the peptide retention from the main branch |
ldk7811
left a comment
ldk7811
left a comment
There was a problem hiding this comment.
Revert the copy commit. This breaks importing the data from 2de. We can always point the next team to a working commit of the zoom.
hold on one second, I'm getting confused. What do I do? Do I revert peptide retention to what it was? I don't know what the copy commit you're referring to is. |
|
you free to hop on a call for a sec? |
|
@ldk7811 It is identical to main. Yesterday you told me it was fine on production, so this should not be functionally different |
ZythiQ
left a comment
ZythiQ
left a comment
There was a problem hiding this comment.
I see no functional differences to peptide retention (if there was a reversion, it was not in this PR).
ShreyesGadwalkar
dismissed
ldk7811’s stale review
Done - file has no changes from main
|
I said it was fine prior to copying data over from main. By copying the peptide retention from main, we now no longer have features specifically from this branch. Namely, you can no longer export proteins to proteolytic digestion. Had it been merged, I would have been annoyed, but zoom could be added back in with relative ease. Please test next time before approving. The feature does not work. |
okay here's what im gonna do next. I'll just have the peptide retention I had in this branch over and open another PR. I was confused by the back and forth. |
* changed acrylamide slider to be a dropdown * transition added * button sizing fixed * added prteolyic Digetion to 2de * starting to integrate jacobs stuff * started pde on this branch * popup UI for pd now works * integrated digestion * font color for grid changed to white * done till next wednesday * added photo * pushed env mistake and removed magic nums * loader added * Re-add basic zoom * pulled peptide retention from main * whitespace push * No changes. * No changes 2x. * final --------- Co-authored-by: jacob-fay <71296174+jacob-fay@users.noreply.github.com> Co-authored-by: Zach Van Horn <zvanhorn10@gmail.com> Co-authored-by: Luke Knofczynski <ldk7811@rit.edu>
7 participants
Combined 2DE and proteolytic digestion. Changed architecture for 2DE into different files.