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

Updated Spectogram.py #82

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Shubhanshu-356
Copy link

PR Description

I have updated the gui responsiveness of Spectogram Application by adding the multithreading, Optimized Plotting and buffering features in it.

Related Issues

Closes #66

Checklist

  • I have gone through the contributing guide
  • I have updated my branch and synced it with the project main branch before making this PR

Undertaking

I declare that:

  • The content I am submitting is original and has not been plagiarized.

  • No portion of the work has been copied from any other source without proper attribution.

  • The work has been checked for plagiarism, and I assure its authenticity.

  • I understand that any violation of this undertaking may have legal consequences that I will bear and could result in the withdrawal of any recognition associated with the work.

  • I Agree

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Great job, @Shubhanshu-356! 🎉 Thank you for submitting your pull request. Your contribution is valuable and we appreciate your efforts to improve our project.

We will promptly review your changes and offer feedback. Keep up the excellent work! Kindly remember to check our contributing guidelines

@Shubhanshu-356
Copy link
Author

Check it once it has no branch conflicts now I have fixed it.

@Shubhanshu-356
Copy link
Author

Check my pull request as I have checked the branching conflicts there is no branching conflicts. Review my changes if possible.

@Soumya-Kushwaha
Copy link
Owner

Check my pull request as I have checked the branching conflicts there is no branching conflicts. Review my changes if possible.

It still shows me conflicts. Please fix it.

@Shubhanshu-356
Copy link
Author

Ok I will fix it and update these as soon as possible.

@Shubhanshu-356
Copy link
Author

Can you please check once. Shall it again showing the same.

Spectogram.py Outdated
import subprocess


main
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you remove branch names mentioned across the file for "main" and "gui-performance-enhancement"

Copy link
Author

@Shubhanshu-356 Shubhanshu-356 Jun 15, 2024

Choose a reason for hiding this comment

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

Means you are saying that I have to remove the branch name
Is it correct I am saying. What is the problem occuring by your side when you are merging. Can you please tell me

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, remove the branch names in the code file. I believe it might've occurred while solving a merge conflict

Copy link
Author

Choose a reason for hiding this comment

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

Ok I will do thanks for the info

Copy link
Author

Choose a reason for hiding this comment

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

Can you please check it now for merging

Copy link
Owner

Choose a reason for hiding this comment

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

could you remove branch names mentioned across the file for "main" and "gui-performance-enhancement"

I still see those!

Copy link
Collaborator

@dinxsh dinxsh left a comment

Choose a reason for hiding this comment

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

LGTM, should be ready to merge

@Shubhanshu-356
Copy link
Author

Thank you

@dinxsh dinxsh requested a review from Soumya-Kushwaha June 15, 2024 08:58
@Shubhanshu-356
Copy link
Author

Shubhanshu-356 commented Jun 16, 2024

But I have checked it's not there now any main
Can you now merge it

@dinxsh
Copy link
Collaborator

dinxsh commented Jun 16, 2024

can still see the branches names mentioned in the file could you remove them and then request for merge @Shubhanshu-356

@Shubhanshu-356
Copy link
Author

Now can you check I have removed all

Copy link
Collaborator

@dinxsh dinxsh left a comment

Choose a reason for hiding this comment

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

fix and kindly test the code before pushing

if __name__ == "__main__":
main()

def close_current_visualizer():
if _VARS["current_visualizer_process"] and _VARS["current_visualizer_process"].poll() is None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

block of code here is missing indentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gssoc level2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve GUI Responsiveness in Spectrogram Application
3 participants