Enable strict=True for zip() where safe #4547
Open
Oll-iver wants to merge 25 commits intoManimCommunity:mainfrom
Open
Enable strict=True for zip() where safe #4547Oll-iver wants to merge 25 commits intoManimCommunity:mainfrom
Oll-iver wants to merge 25 commits intoManimCommunity:mainfrom
Conversation
…o be of the same length.
…d to be of same length.
…tuple has a corresponding datapoint in the second (ie same length)
…rs and forces rgbas and offset to be the same size for strict=True
…rs or strictly forced to be the same length (n). In any case they'll always be the same length so strict=True works.
…us_list is defined directly from the length of vertex_group; both outer_vertices and inner_vertices posess n vertices.
… the zup function are the same length.
…e; labels and parts are seemingly user inputs with no guarantee of equal length; val_range is defined to be same lenght as self.bar_names; however there's no authentication that self.values has a fixed length after it's been defined ie user can append to the list creating a mismatch between len(self.values) and len(self.bars)
…h or manipulated to be by the align_data function. In the match_points function there is currently no validation to ensure both mobjects are the same length.
to strict=False due to failing test cases
…in _add_x_axis_labels we define val_range to be the same length as self.bar_names
… in the zip() function are of the same length. Every other time zip() is used here it is generally immediately manipulating or explicitly defining the tuples to be of same length
…r manipulated to be the same size using the make_even function.
… second tuple is simply a cyclic shift of the first.
…attrs so they are guaranteed to have equal lengths.
…o be equal size or defined to be equal size.
…h tuples as current_number_of_curves is read directly from the shape of bezier_tuples and is used to dictate the size of split_factors.
…th tuples as floors is defined directly from alphas (which also defines alphas_mod1)
…the same length so strict=True.
…o strict=True. While it is technically possible to pass tuples to this function that are *not* the same length, this would result in generally unexpected behaviour anyway.
…bels is dependent on tick_range so guaranteed to have the same length.
…have axis manipulated to be the same length as tick_range. In get_riemann_rectangles() we have colors dependent on of x_range_Array forcing them to be the same length. Finally in plot_line_graph() it is clearly intended that all inputs used in the zip() function are of the same length (except possibly z which may not exist and will be made equal length to x); while it is not guaranteed they will be the same length this would cause unintended behaviour.
…ost test cases either a) hardcode two things to be the same length, b) verify things are the same length before the function or c) explicitly exist to check whether two things are the same length.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview: What does this pull request change?
Changed uses of
zip()tostrict=Truewhere iterables are guaranteed or expected to be of equal length including test suite.Explicitly set strict=False where it had not already been set and truncation is intended.
All remaining uses of strict=False are instances where the
zip()function takes iterables that are not guaranteed or always expected to be of equal length; for example columns and colours in matrix.py.Motivation and Explanation: Why and how do your changes improve the library?
This PR addresses the cleanup task described in Issue #4521.
Future bugs will now raise a
ValueErrorrather than silently proceeding, producing unexpected results, or crashing elsewhere.Some tests were actually ineffective without
strict=Trueas they worked by checking the output ofzip()was the same length as its inputs.Links to added or changed documentation pages
Further Information and Comments
Reviewer Checklist