fix swapped argument required vars in bitmaptools args helper #10253
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.
I believe the intention of this helper function is to have
if_required1
be relvant forx1
andy1
and haveif_required2
be relevant forx2
andy2
.That would mean that it is possible to control the required-ness of x1,y1 and x2,y2 independently.
As it is written now instead
if_required1
is relevant forx1
andx2
andif_required2
is relevant fory1
andy2
. If this functionality does match the intention then I would proposeif_requiredX
andif_requiredY
as more descriptive variable names.But I believe the x1,y1 and x2,y2 delineation was likely the original intention as it's plausible there could be a function that requires one coordinate point, but has the second one be optional. And seems less plausible to me that a function would have x1,x2 required and y1,y2 optional or vice versa for instance.
The only place where this helper function is called that the
if_required
arguments actually differ is here: https://github.com/adafruit/circuitpython/blob/main/shared-bindings/bitmaptools/__init__.c#L722Using this statement which calls the affected function:
under
10.0.0-alpha.2
results in this exception:using this PR branch instead successfully runs as the docstrings indicate it should if x1 and y1 are omitted.
I do also believe there are other issues with the arrayblit() arg validation and/or docstrings, but it's separate from this issue with the swapped values in the helper function here, it just so happens to make use of this.