wincowgerDEV / OpenSpecy-package

Analyze, Process, Identify, and Share, Raman and (FT)IR Spectra
http://wincowger.com/OpenSpecy-package/
Creative Commons Attribution 4.0 International
23 stars 11 forks source link

Manual baseline and smooth #71

Closed wincowgerDEV closed 3 years ago

wincowgerDEV commented 3 years ago

@zsteinmetz I have the new manual baseline correction working pretty well. I am sure there are some UX improvements we could make but we have something to build from with this for now. Try out the new baseline correction tool and let me know what you think.

zsteinmetz commented 3 years ago

Nice work! I really like the new feature.

To you have anything else on your todo or do you consider this ready to squash and merge?

wincowgerDEV commented 3 years ago

I still need to run a test to make sure it is uploading to mongo db and saving to loggit properly but afterwards it should be good.

On Fri, Apr 30, 2021, 2:52 AM Zacharias Steinmetz @.***> wrote:

Nice work! I really like the new feature.

To you have anything else on your todo or do you consider this ready to squash and merge?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/71#issuecomment-829980981, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU2RQHMPWLDDIW7TS7TTLJ4XHANCNFSM433A67KQ .

wincowgerDEV commented 3 years ago

Could be improved by displaying the trace of the baseline fit on the original spectrum for reassurance.

wincowgerDEV commented 3 years ago

Passing all my tests, ready to merge if you are.

zsteinmetz commented 3 years ago

Passing all my tests, ready to merge if you are.

Mine too!

What I just stumbled upon: Is there a way of redrawing the baseline? It seems like the manual correction persists even after switching back to the polynomial corrections. While this may be intended, it might be nice to have an extra button to delete the drawn baseline to start over without being forced to upload the file again.

Should we implement this before merging? What do you think? Sorry for seeing this just now.

wincowgerDEV commented 3 years ago

Hey Zacharias,

I think that is a good idea. I think it would just be another observeEvent that would trigger with the delete button to reset the baseline to NULL but I don't know how that will interact with what people are seeing on the plotly and it might be hard to clear the line. I will look into it a little right now. Probably a good idea to fix that before we merge.

Win

On Mon, May 3, 2021 at 5:37 AM Zacharias Steinmetz @.***> wrote:

Passing all my tests, ready to merge if you are.

Mine too!

What I just stumbled upon: Is there a way of redrawing the baseline? It seems like the manual correction persists even after switching back to the polynomial corrections. While this may be intended, it might be nice to have an extra button to delete the drawn baseline to start over without being forced to upload the file again.

Should we implement this before merging? What do you think? Sorry for seeing this just now.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/71#issuecomment-831231442, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU4HUNHHJCYXW4AQAZTTL2KHRANCNFSM433A67KQ .

--

´¯·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------

Win Cowger PhD Candidate, Environmental Sciences: Soil and Water University of California, Riverside

NSF Graduate Research Fellow Research Advisor to 5 Gyres https://www.5gyres.org/advisors/ Data Advisor to Let's Do It World https://www.letsdoitworld.org/

Contact Info

515-298-3869 | @.***

Websites www.openspecy.org www.wincowger.com http://andrewgray.ucr.edu/people/wcowger.html


wincowgerDEV commented 3 years ago

Ok, now I have a reset button on the manual page which will reset the baseline information to NULL making it not get used. If people switch the mode to polynomial, the manual baseline doesn't transfer over or get added to the polynomial baseline fit in any way since the manual baseline fit is dependant on the manual option being active. The previous line that was drawn does get saved in the background so if a person comes back to the manual dropdown and then clicks correct it will correct with the previous line even though they can't see the line (because the line gets removed when the plot refreshes) but if they draw a new line and then click correct it will only use the new line and they can always click the reset button to see it without the correction. Let me know what you think about this behavior. I think it works pretty well for a first iteration of a new tool. We can add some additional features later but it feels functional to me.

On Mon, May 3, 2021 at 6:30 AM win cowger @.***> wrote:

Hey Zacharias,

I think that is a good idea. I think it would just be another observeEvent that would trigger with the delete button to reset the baseline to NULL but I don't know how that will interact with what people are seeing on the plotly and it might be hard to clear the line. I will look into it a little right now. Probably a good idea to fix that before we merge.

Win

On Mon, May 3, 2021 at 5:37 AM Zacharias Steinmetz < @.***> wrote:

Passing all my tests, ready to merge if you are.

Mine too!

What I just stumbled upon: Is there a way of redrawing the baseline? It seems like the manual correction persists even after switching back to the polynomial corrections. While this may be intended, it might be nice to have an extra button to delete the drawn baseline to start over without being forced to upload the file again.

Should we implement this before merging? What do you think? Sorry for seeing this just now.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/wincowgerDEV/OpenSpecy/pull/71#issuecomment-831231442, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGMUJU4HUNHHJCYXW4AQAZTTL2KHRANCNFSM433A67KQ .

--

´¯·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------

Win Cowger PhD Candidate, Environmental Sciences: Soil and Water University of California, Riverside

NSF Graduate Research Fellow Research Advisor to 5 Gyres https://www.5gyres.org/advisors/ Data Advisor to Let's Do It World https://www.letsdoitworld.org/

Contact Info

515-298-3869 | @.***

Websites www.openspecy.org www.wincowger.com http://andrewgray.ucr.edu/people/wcowger.html


--

´¯·.¸¸.·´¯·.´¯·.¸¸.·´¯ツ ------------------------------

Win Cowger PhD Candidate, Environmental Sciences: Soil and Water University of California, Riverside

NSF Graduate Research Fellow Research Advisor to 5 Gyres https://www.5gyres.org/advisors/ Data Advisor to Let's Do It World https://www.letsdoitworld.org/

Contact Info

515-298-3869 | @.***

Websites www.openspecy.org www.wincowger.com http://andrewgray.ucr.edu/people/wcowger.html


zsteinmetz commented 3 years ago

I like this!