Skip to content

Conversation

@sadrasabouri
Copy link
Member

@sadrasabouri sadrasabouri commented Jul 11, 2023

Reference Issues/PRs

#5

As @NimaSamadi007 said:

What does this implement/fix? Explain your changes.

This PR adds the async functionality to Nava. Along with the implementation, a simple example is provided that shows the expected behavior of the async feature.

Any other comments?

I wanted to integrate sync and async more seamlessly, but I couldn't as it seems to me creating a class that contains the player object could help more. I was hesitant to make significant changes to the base code, as doing so could potentially cause severe problems. I recommend that we have a conversation about the matter soon.

My solution for this part is to define a new function play_async so two parts are completely separated. What do you think @sepandhaghighi ?

And thanks for your efforts @NimaSamadi007

@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (3d3ec51) 100.00% compared to head (d00bd1f) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##               dev       #27   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           56        77   +21     
  Branches         4         6    +2     
=========================================
+ Hits            56        77   +21     
Impacted Files Coverage Δ
nava/functions.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sadrasabouri sadrasabouri marked this pull request as ready for review July 11, 2023 14:51
@sadrasabouri sadrasabouri self-assigned this Jul 11, 2023
@sadrasabouri sadrasabouri added documentation Improvements or additions to documentation enhancement New feature or request new feature labels Jul 11, 2023
@sadrasabouri sadrasabouri added this to the nava v0.3 milestone Jul 11, 2023
@sadrasabouri
Copy link
Member Author

As we discussed with @sepandhaghighi, this approach is not well-suited for the higher-end user. We want something like this:

playsound('test.wav', is_async=True)

and no extra function definition nor using asyncio.run.

The best approach, I believe, is to use threading instead. what do you think @NimaSamadi007?
And do you have any interests working on it (this time based on the text file that has been sent to you as an example)?

@NimaSamadi007
Copy link
Contributor

Yeah, I agree. It's better we hide the internal mechanisms of the library from high-end users.
I will create a new branch and will work on implementing this feature with threading instead. Then, I'll create another PR.

@sadrasabouri
Copy link
Member Author

Closed by opening #28

@sepandhaghighi sepandhaghighi mentioned this pull request Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants