Skip to content

Preserve Class Repr After add_traits#501

Merged
minrk merged 1 commit intomasterfrom
rmorshea-add_traits-repr
Apr 5, 2019
Merged

Preserve Class Repr After add_traits#501
minrk merged 1 commit intomasterfrom
rmorshea-add_traits-repr

Conversation

@rmorshea
Copy link
Contributor

Closes #500. The repr for self.__class__ is not identical after calling add_traits. This PR copies the __module__ and __qualname__ from the original class to the new one.

zerline pushed a commit to sagemath/sage-combinat-widgets that referenced this pull request Oct 24, 2018
Copy link

@zerline zerline left a comment

Choose a reason for hiding this comment

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

This way, a derived class still belongs to the same module after adding traits.

Copy link

@zerline zerline left a comment

Choose a reason for hiding this comment

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

Helps a lot. Will it be merged soon?

@minrk minrk closed this Mar 29, 2019
@minrk minrk reopened this Mar 29, 2019
@rmorshea
Copy link
Contributor Author

rmorshea commented Apr 2, 2019

@minrk I think we should merge this. What is your preferred merge strategy?

@minrk minrk merged commit 7b48947 into master Apr 5, 2019
@minrk
Copy link
Member

minrk commented Apr 5, 2019

Closed and reopened to trigger CI, lgtm

@Carreau Carreau added this to the 5.0 milestone Jun 4, 2020
@Carreau Carreau added 5.0-re-review Need to re-review for potential API impact changes. 5.0-minor rereviewed, minor change need to be put in changelog. labels Jun 4, 2020
@Carreau Carreau removed the 5.0-re-review Need to re-review for potential API impact changes. label Jun 22, 2020
@Carreau Carreau deleted the rmorshea-add_traits-repr branch August 17, 2021 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5.0-minor rereviewed, minor change need to be put in changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Why a new class?

4 participants