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

Ensure last_read is always set. #2257

Merged
merged 1 commit into from
Nov 24, 2024
Merged

Conversation

Breakthrough
Copy link
Contributor

@Breakthrough Breakthrough commented Nov 23, 2024

The property lastread was renamed to last_read in #1170 however not all instances were changed. This leads to some cases where only one of the variables is updated. For example, in the case where one calls get_frame with a frame exactly one ahead of the current position. In this case, only self.last_read is updated, but self.lastread remains unchanged, leading the two properties to get out of sync.

There are third-party projects which relied on the lastread property as it was never declared as private in previous versions (e.g. no underscore prefix). To avoid breaking projects relying on this, we also provide a property with the old name for backwards compatibility.

This should also help maintain the code, as there is already some confusion over the two similarly named properties (#2044).

  • I have provided code that clearly demonstrates the bug and that only works correctly when applying this fix
  • I have properly documented new or changed features in the documentation or in the docstrings
  • I have properly explained unusual or unexpected code in the comments around it

The property `lastread` was renamed to `last_read` in Zulko#1170 however
not all instances were changed. This leads to some cases where only
one of the variables is updated, which can make detection of end-of-file
more complicated than it used to be in MoviePy 1.x.

Additionally, there are third-party projects which relied on the lastread
property as it was never declared as private (e.g. no underscore prefix).
To avoid breaking projects relying on this, we also provide a property
with the old name for backwards compatibility.

Related to Zulko#2044
@Breakthrough
Copy link
Contributor Author

Passes unit tests locally and ensured code is formatted.

@coveralls
Copy link

Coverage Status

coverage: 86.35% (-0.08%) from 86.428%
when pulling 943217d on Breakthrough:issue-2044
into c88852f on Zulko:master.

@Zulko
Copy link
Owner

Zulko commented Nov 24, 2024

Thank you! Will do a new release on Sunday.

@Zulko Zulko merged commit fcc580c into Zulko:master Nov 24, 2024
7 of 15 checks passed
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.

3 participants