close
The Wayback Machine - https://web.archive.org/web/20260215150056/https://github.com/TheAlgorithms/Python/pull/3679
Skip to content

added text_to_speech#3679

Closed
asperduti wants to merge 8 commits intoTheAlgorithms:masterfrom
asperduti:text_to_speech
Closed

added text_to_speech#3679
asperduti wants to merge 8 commits intoTheAlgorithms:masterfrom
asperduti:text_to_speech

Conversation

@asperduti
Copy link

@asperduti asperduti commented Oct 23, 2020

Describe your change:

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Please provide tests from the following, whichever you prefer:
doctest, unittest, pytest

@dhruvmanila dhruvmanila added require tests Tests [doctest/unittest/pytest] are required awaiting changes A maintainer has requested changes to this PR labels Nov 9, 2020
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

You would also require to write tests for the function. This would be a bit difficult so if you face any problem, ping me, but before that please try first.

@ghost ghost added awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass and removed require tests Tests [doctest/unittest/pytest] are required awaiting changes A maintainer has requested changes to this PR awaiting reviews This PR is ready to be reviewed labels Nov 26, 2020
@ghost ghost added awaiting reviews This PR is ready to be reviewed and removed tests are failing Do not merge until tests pass labels Nov 26, 2020
@asperduti
Copy link
Author

asperduti commented Nov 26, 2020

You would also require to write tests for the function. This would be a bit difficult so if you face any problem, ping me, but before that please try first.

Hi @dhruvmanila
Well, after some troubles, I did it. Please tell me if you need more tests or something else.

I structured the code differently, I split it up to make easier the testing.

@ghost ghost added awaiting changes A maintainer has requested changes to this PR and removed awaiting reviews This PR is ready to be reviewed labels Dec 10, 2020
@ghost ghost added awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass and removed awaiting changes A maintainer has requested changes to this PR awaiting reviews This PR is ready to be reviewed labels Dec 11, 2020
@ghost ghost added awaiting reviews This PR is ready to be reviewed and removed tests are failing Do not merge until tests pass labels Dec 11, 2020
@asperduti
Copy link
Author

Hi @dhruvmanila I worked on all the requested changes.

@stale
Copy link

stale bot commented Jan 11, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Used to mark an issue or pull request stale. label Jan 11, 2021
@asperduti
Copy link
Author

Hi, @ xcodz-dot , @dhruvmanila .

Did you check it?
I worked in the things that you told me.

@stale stale bot removed the stale Used to mark an issue or pull request stale. label Jan 11, 2021
Copy link

@ghost ghost 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

@stale
Copy link

stale bot commented Feb 13, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Used to mark an issue or pull request stale. label Feb 13, 2021
@ghost
Copy link

ghost commented Feb 16, 2021

cc: @cclauss

@stale stale bot removed the stale Used to mark an issue or pull request stale. label Feb 16, 2021
@dhruvmanila
Copy link
Member

Hi, I will take a look at this in a day or two.

@dhruvmanila dhruvmanila self-requested a review February 17, 2021 13:55
Comment on lines +12 to +32
html = "<p>Listen to the MP3 file :<BR></p> <!> \
<object classid='clsid:D27CDB6E-AE6D-11cf-96B8-444553540000' \
width='470' height='24' id='single1' name='single1'> \
<param name='movie' value='player.swf'> \
<param name='allowfullscreen' value='true'> \
<param name='allowscriptaccess' value='always'> \
<param name='wmode' value='transparent'> \
<param name = 'flashvars' \
value='file=/output/0360061001606401513/59405670.mp3'> \
<embed \
id='single2' \
name='single2' \
src='player.swf' \
width='470' \
height='24' \
bgcolor='#000000' \
allowscriptaccess='always' \
allowfullscreen='true' \
flashvars='file=/output/0360061001606401513/59405670.mp3' \
/> \
</object>"
Copy link
Member

Choose a reason for hiding this comment

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

Triple-quoted multiline strings work in Python and as PEP8 says, backslashes should be avoided in Python code.

Suggested change
html = "<p>Listen to the MP3 file :<BR></p> <!> \
<object classid='clsid:D27CDB6E-AE6D-11cf-96B8-444553540000' \
width='470' height='24' id='single1' name='single1'> \
<param name='movie' value='player.swf'> \
<param name='allowfullscreen' value='true'> \
<param name='allowscriptaccess' value='always'> \
<param name='wmode' value='transparent'> \
<param name = 'flashvars' \
value='file=/output/0360061001606401513/59405670.mp3'> \
<embed \
id='single2' \
name='single2' \
src='player.swf' \
width='470' \
height='24' \
bgcolor='#000000' \
allowscriptaccess='always' \
allowfullscreen='true' \
flashvars='file=/output/0360061001606401513/59405670.mp3' \
/> \
</object>"
html = """<p>Listen to the MP3 file :<BR></p> <!>
<object classid='clsid:D27CDB6E-AE6D-11cf-96B8-444553540000'
width='470' height='24' id='single1' name='single1'>
<param name='movie' value='player.swf'>
<param name='allowfullscreen' value='true'>
<param name='allowscriptaccess' value='always'>
<param name='wmode' value='transparent'>
<param name = 'flashvars'
value='file=/output/0360061001606401513/59405670.mp3'>
<embed
id='single2'
name='single2'
src='player.swf'
width='470'
height='24'
bgcolor='#000000'
allowscriptaccess='always'
allowfullscreen='true'
flashvars='file=/output/0360061001606401513/59405670.mp3'
/>
</object>"""

import requests

LANGUAGES_VOICES = {
"US English": ["Alice", "Daisy", "George", "Jenna", "John"],
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"US English": ["Alice", "Daisy", "George", "Jenna", "John"],
"US English": ("Alice", "Daisy", "George", "Jenna", "John"),

Tuples take less RAM than lists and they indicate to the reader that the values will not change at runtime.

Comment on lines +50 to +55
if match := re.search(r"\/output\/\d+\/\d+\.mp3", html):
filename = match.group()
else:
filename = None

return filename
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if match := re.search(r"\/output\/\d+\/\d+\.mp3", html):
filename = match.group()
else:
filename = None
return filename
match = re.search(r"\/output\/\d+\/\d+\.mp3", html):
return match.group() if match else None

}


def get_mp3_filename(html: str) -> Optional[str]:
Copy link
Member

Choose a reason for hiding this comment

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

Why two different return types? Why not just return an empty str instead of None?

Comment on lines +123 to +125
audio_url = f"http://www.fromtexttospeech.com{mp3_file}"

mp3data = requests.get(audio_url)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
audio_url = f"http://www.fromtexttospeech.com{mp3_file}"
mp3data = requests.get(audio_url)
mp3data = requests.get(f"http://www.fromtexttospeech.com{mp3_file}")

Copy link
Member

Choose a reason for hiding this comment

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

This is not really mp3data. Instead, it is a requests.Response so perhaps response would be a better variable name than mp3data.

Comment on lines +127 to +133
# save the data as an mp3 file
if mp3data.status_code != 404:
with open(audio_path, "w+b") as f:
f.write(mp3data.content)
return True

return False
Copy link
Member

@cclauss cclauss Feb 17, 2021

Choose a reason for hiding this comment

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

Suggested change
# save the data as an mp3 file
if mp3data.status_code != 404:
with open(audio_path, "w+b") as f:
f.write(mp3data.content)
return True
return False
try:
response.raise_for_status()
# save the data as an mp3 file
with open(audio_path, "wb") as f:
f.write(response.content)
return True
except requests.HTTPError:
return False

self.assertEqual("/output/0360061001606401513/59405670.mp3", mp3_file_name)

def test_get_mp3_file_invalid_html(self):
html = "<p>Listen to the MP3 file :<BR></p> <!> \
Copy link
Member

Choose a reason for hiding this comment

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

Triple-quoted str with no backslashes please.

@stale
Copy link

stale bot commented Mar 19, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Used to mark an issue or pull request stale. label Mar 19, 2021
@stale
Copy link

stale bot commented Jun 5, 2021

Please reopen this pull request once you commit the changes requested or make improvements on the code. If this is not the case and you need some help, feel free to seek help from our Gitter or ping one of the reviewers. Thank you for your contributions!

@stale stale bot closed this Jun 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reviews This PR is ready to be reviewed stale Used to mark an issue or pull request stale.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants