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

Issue #126 - removed inheritance from object and removed return value… #127

Merged
merged 1 commit into from
Sep 24, 2015
Merged

Issue #126 - removed inheritance from object and removed return value… #127

merged 1 commit into from
Sep 24, 2015

Conversation

sbonnick
Copy link
Contributor

Issue #126 - removed inheritance from object and removed return values in comments as both are considered bad form

Not sure yet if this change will fully fix my issue. but it should be changed anyway to be more "pythonic".

All tests pass.

…s in comments as both are considered bad form

Not sure yet if this change will fully fix my issue. but it should be
changed anyway to be more "pythonic". All tests pass.
bear added a commit that referenced this pull request Sep 24, 2015
Issue #126 - removed inheritance from object and removed return value…
@bear bear merged commit b1657cc into bear:master Sep 24, 2015
@idpaterson
Copy link
Collaborator

I am curious, why would inheritance from object be considered bad form? In Python 3 the inheritance from object is implicit but in Python 2 there are significant differences between old-style classes and new-style (inheriting object) classes. Is there a specific issue that this addresses? Otherwise it seems to create an unnecessary incompatibility between Python 2 and 3.

@philiptzou
Copy link
Collaborator

I also don't agree to remove inheritance from object unless we want to drop Python 2 support progressively.

@sbonnick
Copy link
Contributor Author

sorry, I do not mean inheritance from object is bad form. I meant that for the @rtype/@return doc tag - and by bad form I mean classes don't need to list their return type by many style guides. Google style guide being one: http://sphinxcontrib-napoleon.readthedocs.org/en/latest/example_google.html

for removing object, I was unaware that there was a difference in python 2 between the two class styles. So after researching this, I would agree that it may be best to change this back to inheriting from object for any consumers using python 2. I will make another PR.

The problem i was attempting to solve was #126 which Pycharm was detecting the return object to be of type "object" and therefore not recognizing the "parse" definition.

@bear
Copy link
Owner

bear commented Sep 24, 2015

Let's add back the object inheritance item and remove the rtype flag

@philiptzou
Copy link
Collaborator

@bear the rtype should be changed to Calendar and I think it will work in PyCharm.

@sbonnick
Copy link
Contributor Author

do we want it there at all though? it conveys no extra meaning as this is on a class.

I still agree with object going back in. if you want rtype back but with Calendar I will add that too but think its unnecessary on a class.

@bear
Copy link
Owner

bear commented Sep 24, 2015

Let's add rtype with Calendar and see if that fixes the issue - that would be the smallest change to address the problem.

@sbonnick sbonnick mentioned this pull request Sep 24, 2015
@sbonnick
Copy link
Contributor Author

pull request created. Thanks guys for spotting the problem with removing object inheritance.

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.

4 participants