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

Update super usage #1280

Merged
merged 2 commits into from
Aug 26, 2020
Merged

Update super usage #1280

merged 2 commits into from
Aug 26, 2020

Conversation

rahulporuri
Copy link
Contributor

This PR cleans up the usage of super - I noticed that the codebase was a mix of super(ClassName, self) and super() calls, which I have updated to use super() in this PR.

The update was done using regex-search and replace. The regex string that works for super(ClassName, self) is (super)+\(+(\w+)(,)+\s(self)+\)+, learnt/debugged using https://regex101.com/.

After making changed using the automated search/replace, the changes were manually checked using git add -p *.

Finally, I manually searched the codebase for uses of super which the regex didn't match e.g. super( ClassName , self ) or super(ClassName,self) - which were manually updated.

Checklist

  • Tests
  • Update API reference (docs/source/traits_api_reference)
  • Update User manual (docs/source/traits_user_manual)
  • Update type annotation hints in traits-stubs

@mdickinson
Copy link
Member

@rahulporuri: where did you find this one?

super( ClassName , self )

I was hoping that after we'd blackened the codebase, all cases of this sort of spacing were gone. (And then that does indeed help with search-and-replace-type refactorings.)

@rahulporuri
Copy link
Contributor Author

@rahulporuri: where did you find this one?

docs

Sai Rahul Poruri added 2 commits August 26, 2020 08:32
done using automated search-replace using regex
	modified:   docs/source/traits_user_manual/advanced.rst
	modified:   traits/has_traits.py
Copy link
Member

@mdickinson mdickinson left a comment

Choose a reason for hiding this comment

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

LGTM

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.

None yet

2 participants