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

remove_overlaps and to_string #70

Closed
wants to merge 6 commits into from
Closed

remove_overlaps and to_string #70

wants to merge 6 commits into from

Conversation

doakey3
Copy link

@doakey3 doakey3 commented Apr 15, 2017

I've created 2 new functions. 1 for removing the overlap between subtitles and another for getting the srt format as a string (as opposed to saving to a file).

@coveralls
Copy link

coveralls commented Apr 15, 2017

Coverage Status

Coverage decreased (-0.2%) to 71.128% when pulling abdfe9f on doakey3:master into 529d83e on byroot:master.

@doakey3
Copy link
Author

doakey3 commented Apr 15, 2017

In my remove_overlaps function, I made it automatically call clean_indexes() if a subtitle section gets entirely removed.

I noticed in one of your comments you said that this could cause a problem for certain types of srt files. If this is the case, then I should alter my function to not automatically clean the indexes. EDIT: Done.

@coveralls
Copy link

coveralls commented Apr 15, 2017

Coverage Status

Coverage increased (+1.1%) to 72.414% when pulling 21b9a6c on doakey3:master into 529d83e on byroot:master.

@coveralls
Copy link

coveralls commented Apr 15, 2017

Coverage Status

Coverage increased (+1.1%) to 72.414% when pulling 81b49bd on doakey3:master into 529d83e on byroot:master.

@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Coverage increased (+1.1%) to 72.414% when pulling f282b20 on doakey3:master into 529d83e on byroot:master.

@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Coverage increased (+1.1%) to 72.414% when pulling eb37d91 on doakey3:master into 529d83e on byroot:master.

@byroot
Copy link
Owner

byroot commented Apr 24, 2017

Sorry but I don't think remove_overlap belongs to pysrt. It's a very opinionated method that could legitimately behave in multiple different ways. I prefer to let pysrt users implement that themselves.

As for to_string, I'm pretty sure ''.join(subtitles) should works.

@byroot byroot closed this Apr 24, 2017
@nipunasudha
Copy link

nipunasudha commented Mar 14, 2021

Although this PR didn't make it into the main repo, this helped me a lot with my project. Thank you very much @doakey3!

@byroot you are right to reject this PR, even for me this was not the overlap removal method I needed. This changes start times of subtitles, but what actually made sense is to shorten lifetime of subtitles by changing the end time.

@ghost
Copy link

ghost commented Aug 14, 2022

As for to_string, I'm pretty sure ''.join(subtitles) should works.

That doesn't seem to be the case.

>>> subtitles = pysrt.from_string('1\n00:00:00,000 --> 00:00:05,000\nTest')
>>> ''.join(subtitles)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: sequence item 0: expected str instance, SubRipItem found

This works though:

>>> subtitles = pysrt.from_string('1\n00:00:00,000 --> 00:00:05,000\nTest\n\n2\n00:00:10,000 --> 00:00:15,000\nTest 2')
>>> '\n'.join(map(str, subtitles))
'1\n00:00:00,000 --> 00:00:05,000\nTest\n\n2\n00:00:10,000 --> 00:00:15,000\nTest 2\n'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants