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

Resolving Issue #699 - Fix runtime error #712

Merged
merged 22 commits into from
Jun 19, 2021

Conversation

lorena-b
Copy link
Member

@lorena-b lorena-b commented Jun 8, 2021

Motivation and Context

This pull request is required to resolve #699. This change resolves a runtime error which occurs when running the UnnecessaryIndexingChecker on a block of code where an assignment node has an empty scope.

This pull request also fixes functions that no longer worked correctly due to changes in the astroid node representation, which caused the example file cases to fail.

Your Changes

Description:

This change handles the case where the second argument of the tuple returned by node.lookup() is an empty tuple so that an index error does not occur.

_is_load_subscript() helper function was rewritten to produce the correct output.

Type of change (select all that apply):

  • Bug fix (non-breaking change which fixes an issue)

Testing

In order to test this change a test suite was created for this checker using pylints testing capabilities. The checker was tested with the example code and the code that raised the original error.

Questions and Comments (if applicable)

The tests for the code examples are failing because the checker seems to not have been implemented correctly. I am trying to figure out what is wrong with the current implementation that makes the checker not work even in basic cases.

Update:

  • There was an issue with the _is_load_subscript() helper function due to changes in the astroid node representation. When indexing the iterable the Name node now directly has a Subscript as the parent, instead of having an Index node in between.
  • Checker now works to pass all of the example cases, issue unnecessary-indexing runtime error #699 case, and some extra cases such as unused iteration variables and looping through the iterable directly.

Checklist

  • I have performed a self-review of my own code.
  • I have verified that the TravisCI tests have passed.
  • I have reviewed the test coverage changes reported on Coveralls.
  • I have added tests for my changes.

Copy link
Contributor

@david-yz-liu david-yz-liu left a comment

Choose a reason for hiding this comment

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

@lorena-b great detective work. For the issue of the checker no longer working, you determined that the Index node is no longer available, and replaced that with accessing the Subscript directly.

However, this is a change in Python 3.9, so the tests are now failing in Python 3.8. In cases like this, we can check sys.version (imported from the sys module) to see what the current version of Python is, and then based on the result execute different branches of code. You should do this for the is_load_subscript function.

python_ta/checkers/unnecessary_indexing_checker.py Outdated Show resolved Hide resolved
python_ta/checkers/unnecessary_indexing_checker.py Outdated Show resolved Hide resolved
python_ta/checkers/unnecessary_indexing_checker.py Outdated Show resolved Hide resolved
@lorena-b lorena-b marked this pull request as ready for review June 19, 2021 00:49
@david-yz-liu david-yz-liu merged commit 3aa6f24 into pyta-uoft:master Jun 19, 2021
@lorena-b lorena-b deleted the issue-699 branch June 20, 2021 19:13
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.

unnecessary-indexing runtime error
2 participants