[proxy] github.com← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public

Conversation

Copy link
Contributor

wbolster commented Jul 20, 2017

Copy link

@wbolster, thanks for your PR! By analyzing the history of the files in this pull request, we identified @serhiy-storchaka, @Yhg1s and @warsaw to be potential reviewers.

wbolster changed the title bpo30977: define uuid.UUID.__slots__ to save memory 30977: define uuid.UUID.__slots__ to save memory Jul 20, 2017
tiran changed the title 30977: define uuid.UUID.__slots__ to save memory bpo-30977: define uuid.UUID.__slots__ to save memory Sep 7, 2017
tiran requested a review from warsaw September 15, 2017 11:19
Copy link
Member

tiran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sensible to me. What do you think, @warsaw ?

Copy link

Will this get backported to 2.7.x?

Copy link
Contributor Author

hi, author here. is there anything i can do to help move this pr forward?

Copy link
Member

serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change breaks pickle. See my comments on the tracker.

Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I didn't expect the Spanish Inquisition!. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Copy link
Member

vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please add a test in test_exceptions() to make sure that is_safe attribute cannot be modified, near the "badtype(lambda: setattr(u, 'int', i))" test.

self.__dict__['int'] = int
self.__dict__['is_safe'] = is_safe
super().__setattr__('int', int)
super().__setattr__('is_safe', is_safe)
Copy link
Member

vstinner Sep 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to use object.__setattr__(self, 'int', int).

Copy link
Member

@serhiy-storchaka: "This change breaks pickle. See my comments on the tracker."

Oh, I missed this comment. It should be fixed.

Please rebase also your change.

Copy link
Contributor

taleinat commented Sep 6, 2018

Since this has lingered for a long time with no word from the original author, I've gone ahead and implemented appropriate pickle/unpickle logic. I've created a new PR (#9078) since this one is old and has merge conflicts.

I suggest closing this.

vstinner closed this Sep 6, 2018
Copy link
Contributor Author

wbolster commented Sep 6, 2018

thanks all, especially @taleinat for finishing my first attempt!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants