search

bjoernricks / python-quilt

A quilt implementation in python
MIT License
10 stars 3 forks source link

Implement Patch.__ne__ for Python 2 #19

Closed vadmium closed 7 years ago

vadmium commented 7 years ago

This is a subtle problem that I only exposed when writing a test case for something else. A test function I was trying assumed that _patcha != _patchb would be consistent with _patcha == _patchb. But in Python 2, the != operation was not implemented, and two patch objects that were supposed to be equivalent would be seen as unequal.

Also factor out the Python 2 compatibility into quilt.utils._EqBase.

bjoernricks commented 7 years ago

I am really not sure if _EqBase should be a "private class" as indicatined with the underscore. For now I am ok with it. But maybe in future a more common base class should be introduced which uses _EqBase as a mixin.

vadmium commented 7 years ago

Just to be clear, I use a leading underscore to indicate that something (class, function, constant, etc) is an implementation detail, rather than a public API of the library. So in this case if we decide to change or remove the quilt.utils._EqBase class, there should not be a compatibility concern with external code depending on it. I figure it is safe to mark something as being internal, but as soon as you mark it as a public API, it is less safe to take it back.

bjoernricks commented 7 years ago

Yes that's my understanding of using underscore names too. Functions, methods, classes, arguments, ... starting with underscore are not part of a "public" api and can be removed every time in future.