search

zopefoundation / BTrees

Other
80 stars 28 forks source link

Fix issue 170 #172

Closed jamadden closed 3 years ago

jamadden commented 3 years ago

There are two commits. The first commit enables running all the tests for fsBTrees; it's because the tests didn't run that we didn't catch this error. It's almost entirely small textual changes to coerce the numbers used in tests to the bytes that work with the btrees.

The second commit removes the custom fsBTree.py file and makes the fsBTree module get built with the _module_builder.py functions just like everything else. This also fixes the interfaces and ABCs for these objects.

I'll do a separate PR (based on this one) to address #171.

d-maurer commented 3 years ago

Jason Madden wrote at 2021-6-8 03:58 -0700: @.*** commented on this pull request.

  • def toString(self):
  • return b''.join(self._keys) + b''.join(self._values)

On the contrary, it's a method so self is the correct parameter name. In addition, using self enables linters to see that, yes, in fact, it's ok that this function is using private attributes (_keys, _values) of the paramater.

It will become a method - at this place, it is only a function: I had some difficulties to understand self._keys and self._values which make only sense in the context of a tree/bucket and not in that of a datatype. It took me some moments to recognize that those references to self are not the self of the defined method.

jamadden commented 3 years ago

Thanks for the review!