Skip to content

Conversation

@shivapbhusal
Copy link
Contributor

No description provided.

@jmadler
Copy link
Contributor

jmadler commented May 7, 2019

Thanks for helping address #432 :)

@jmadler jmadler merged commit 923622a into PythonCharmers:master May 7, 2019
@ethanfurman
Copy link

This patch was committed before you heard back from the Python Dev team? That seems premature.

It's also a useless test since the only thing stringy about OurCustomString is the name. To be a correct test you need to add the __eq__ method that Max-Volger showed in his example:

def __eq__(self, other):
    if isinstance(other, (str, newstr)):
        return self.string == other
    else:
        return NotImplemented

You should probably also add a test to make sure OurCustomString compares correctly to a regular string:

self.assertTrue(our_str == 'foobar')

@jmadler
Copy link
Contributor

jmadler commented May 8, 2019

Hi @ethanfurman! I asked @shivapbhusal to write a unit test to confirm the behavior of the design of the function until the design was able to be discussed. I'm sorry this wasn't shared publicly. If you have further comment after the changes made since that conversation has ended please file a new bug.

@shivapbhusal thanks for your first open source contribution. You've done a great job and I look forward to hearing more from you :) See you at the next PyCon!

@shivapbhusal
Copy link
Contributor Author

Hi @jmadler, Thanks a lot for helping me get started. I look forward to making more contributions in the future. See you at the next PyCon!

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.

3 participants