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

Fix TypeError: 'str' object is not callable in LocalPath.__fspath__ #522

Merged
merged 3 commits into from
Dec 11, 2020

Conversation

moskupols
Copy link
Contributor

@moskupols moskupols commented Nov 12, 2020

So basically there was a bug in LocalPath.__fspath__: we tried to call self._path() where _path is a property.
I solved the problem by just inheriting the implementation from Path, as Path.__fspath__ is implemented correctly:

    def __fspath__(self):                                                                                                                          
        """Added for Python 3.6 support"""                                                                                                         
        return str(self)    

@moskupols moskupols changed the title Inherit implementation of __fspath__ in LocalPath from Path Fix TypeError: 'str' object is not callable in LocalPath.__fspath__ Nov 12, 2020
@henryiii
Copy link
Collaborator

Assuming it passes tests, this looks fine to me. Thanks!

@coveralls
Copy link

coveralls commented Nov 12, 2020

Coverage Status

Coverage decreased (-0.2%) to 28.915% when pulling d30298a on moskupols:fix-local-path-fspath into 2976a0c on tomerfiliba:master.

@moskupols
Copy link
Contributor Author

moskupols commented Nov 12, 2020

The tests failed, but I don't think it's because of my patch: seems like there's a problem involving pypy, paramiko, and OpenSSL 1.0.2 no longer being supported.

@moskupols
Copy link
Contributor Author

Created a separate issue for the build problem: #523

@henryiii henryiii closed this Dec 10, 2020
@henryiii henryiii reopened this Dec 10, 2020
@henryiii henryiii merged commit b8c7c34 into tomerfiliba:master Dec 11, 2020
@henryiii henryiii added this to the v1.7.0 milestone Feb 8, 2021
@henryiii
Copy link
Collaborator

henryiii commented Feb 8, 2021

Now questioning to myself why we needed this on LocalPath if Path already had it? @koreno I think you added #498? Not that it hurts.

@koreno
Copy link
Collaborator

koreno commented Feb 10, 2021

Now questioning to myself why we needed this on LocalPath if Path already had it? @koreno I think you added #498? Not that it hurts.

not me, @KOLANICH did. indeed it seems redundant, since it's already on the base class since 2b5a82a

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