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.

Sign up for GitHub

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

Jump to bottom

feature to download the spectrum file (.spec) #619

Merged
merged 30 commits into from
Aug 22, 2022
Merged

feature to download the spectrum file (.spec) #619

merged 30 commits into from
Aug 22, 2022

Conversation

arunavabasucom
Copy link
Owner

@arunavabasucom arunavabasucom commented Aug 3, 2022 •
edited by erwanp
Loading

Fixes #211
Fixes #626
Fixes #571

New features :

  • Working slit function
Values molecule Image
5 mm slit 2000-2300 co Screenshot 2022-08-19 at 4 49 19 PM
15 mm slit 2000-2300 co Screenshot 2022-08-19 at 4 48 47 PM
  • Download spectrum

Screenshot 2022-08-19 at 9 16 11 PM

@arunavabasucom arunavabasucom requested a review from erwanp August 3, 2022 15:38
@arunavabasucom arunavabasucom self-assigned this Aug 3, 2022
@arunavabasucom arunavabasucom changed the title feature to download the spectrum feature to download the spectrum file (.spec) Aug 3, 2022
suzil
suzil reviewed Aug 3, 2022
View reviewed changes
server/main.py Outdated Show resolved Hide resolved
suzil
suzil reviewed Aug 9, 2022
View reviewed changes
server/main.py Outdated Show resolved Hide resolved
@erwanp erwanp mentioned this pull request Aug 10, 2022
Open
@arunavabasucom arunavabasucom marked this pull request as ready for review August 14, 2022 15:57
@arunavabasucom arunavabasucom requested a review from suzil August 14, 2022 15:58
arunavabasucom
arunavabasucom commented Aug 14, 2022
View reviewed changes
server/main.py Outdated Show resolved Hide resolved
suzil
suzil reviewed Aug 14, 2022
View reviewed changes
server/main.py Outdated Show resolved Hide resolved
suzil
suzil reviewed Aug 14, 2022
View reviewed changes
server/main.py Outdated Show resolved Hide resolved
suzil
suzil reviewed Aug 14, 2022
View reviewed changes
server/main.py Outdated Show resolved Hide resolved
suzil
suzil reviewed Aug 15, 2022
View reviewed changes
server/main.py Outdated Show resolved Hide resolved
suzil
suzil reviewed Aug 15, 2022
View reviewed changes
server/main.py Outdated Show resolved Hide resolved
@arunavabasucom arunavabasucom requested review from suzil and erwanp August 15, 2022 15:40
@suzil suzil mentioned this pull request Aug 16, 2022
Open
@erwanp erwanp mentioned this pull request Aug 18, 2022
Open
suzil
suzil reviewed Aug 18, 2022
View reviewed changes
server/main.py Outdated Show resolved Hide resolved
suzil
suzil reviewed Aug 18, 2022
View reviewed changes
Copy link
Collaborator

@suzil suzil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just a couple of small things left but I think we're close to merging 🙂

@arunavabasucom arunavabasucom requested a review from suzil August 19, 2022 15:37
@arunavabasucom
Copy link
Owner Author

👉 References :

CleanShot.2022-08-19.at.20.38.00.mp4

Screenshot 2022-08-19 at 9 16 11 PM

@arunavabasucom
Copy link
Owner Author

arunavabasucom commented Aug 19, 2022 •
edited
Loading

Values molecule Image
5 mm slit 2000-2300 co Screenshot 2022-08-19 at 4 49 19 PM
15 mm slit 2000-2300 co Screenshot 2022-08-19 at 4 48 47 PM

@suzil
Copy link
Collaborator

suzil commented Aug 21, 2022 •
edited
Loading

I think the UI behavior of enabling the button can be a little confusing to the user if we fail to plot a spectrum for a given input. If the user inputs something that fails to plot, the download button still becomes enabled and you can still download something.

Screen.Recording.2022-08-21.at.8.09.44.AM.mov

Maybe we could disable the download button if something isn't plotted OR if the user changes the input form after something gets plotted. Thoughts @erwanp and @arunavabasu-03 ?

@erwanp
Copy link
Collaborator

erwanp commented Aug 22, 2022

I had suggested this, is it possible ? IT would fix your problem

Just turn it gray whenever you press Calculate.
Then visible when the spectrum is calculated and shown.
Then gray again when Calculate is pressed again.

#619 (comment)

@arunavabasucom
Copy link
Owner Author

arunavabasucom commented Aug 22, 2022 •
edited
Loading

Ahh @erwanp !! @suzil is saying that based on the graph plotted or not disable or enable the download button.

@erwanp
Copy link
Collaborator

erwanp commented Aug 22, 2022

Disabling download after something gets plotted is good; but it's going to take time, and we need to have these changes Merged and tested before Wednesday. Just do Disable download during a calculation and until the graph is plotted for the moment

@arunavabasucom arunavabasucom requested a review from suzil August 22, 2022 12:41
suzil
suzil approved these changes Aug 22, 2022
View reviewed changes
Copy link
Collaborator

@suzil suzil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Great work on this feature @arunavabasu-03

@suzil suzil merged commit 2b754ff into arunavabasucom:main Aug 22, 2022
@erwanp erwanp mentioned this pull request Aug 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@suzil suzil suzil approved these changes

@erwanp erwanp Awaiting requested review from erwanp

Assignees

@arunavabasucom arunavabasucom

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Molecule label bug Instrumental slit doesn't work Idea: Download Spectrum for further analysis
3 participants
@arunavabasucom @suzil @erwanp