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

Infixowl cleanup iii #2033

Merged
merged 5 commits into from Jul 18, 2022
Merged

Infixowl cleanup iii #2033

merged 5 commits into from Jul 18, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jul 17, 2022

Maintenance-positive changes recommended by flake8-bugbear, mostly explicitly indicating unused loop variables with a prefixing underscore (it identiified one case where none of the loop variables were being used) and a couple of don't-use-mutable-objects-as-args instances.

Summary of changes

  1. Added leading underscore to unused loop variables, removed mutable objects from args, (replacing with None and then performing an explicit “if None” assignment in the body of the method).

  2. Swapped out an excessively weak, coverage-motivated length test of hash() (which was causing intermittent test failures) for an equality test of two runs, fixes issue Low probability failure in test/test_extras/test_infixowl/test_restriction.py::test_restriction_str_and_hash #2030

Checklist

  • Checked that there aren't other open pull requests for the same change.
  • Added tests for any changes that have a runtime impact.
  • Checked that all tests and type checking passes.
  • For changes that have a potential impact on users of this project:
    • Updated relevant documentation to avoid inaccuracies.
    • Considered adding additional documentation.
    • Considered adding an example in ./examples for new features.
    • Considered updating our changelog (CHANGELOG.md).
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

@coveralls
Copy link

coveralls commented Jul 17, 2022

Coverage Status

Coverage remained the same at 90.445% when pulling 867fe55 on gjhiggins:infixowl-cleanup-iii into 40eaa26 on RDFLib:master.

@ghost ghost self-requested a review July 17, 2022 18:48
Copy link
Member

@aucampia aucampia left a comment

Choose a reason for hiding this comment

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

LGMT, also updated the CHANGELOG.

@aucampia aucampia requested a review from a team July 17, 2022 21:55
@aucampia aucampia merged commit 8d5a14f into RDFLib:master Jul 18, 2022
@ghost ghost deleted the infixowl-cleanup-iii branch July 18, 2022 17:59
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.

Low probability failure in test/test_extras/test_infixowl/test_restriction.py::test_restriction_str_and_hash
2 participants