Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update 'inactive_overload_cache' key #2092

Closed
wants to merge 1 commit into from

Conversation

BetsyMcPhail
Copy link

@BetsyMcPhail BetsyMcPhail commented Jan 23, 2020

Instead of just the function name, use __qualname__.name.

Solves issue #1922

@wjakob

Instead of just the function name, use '__qualname__.name'.
@EricCousineau-TRI
Copy link
Collaborator

@YannickJadoul @rwgk I was updating our fork, and forgot that we hadn't landed this (see linked issue for more deets).

Is this a candidate for 2.6.0?

@EricCousineau-TRI
Copy link
Collaborator

Came across this when crafting: RobotLocomotion#47

@YannickJadoul
Copy link
Collaborator

This should probably use detail::get_fully_qualified_tp_name (from #2520), but apart from that, a quick look says "yes, I think so?"

@YannickJadoul
Copy link
Collaborator

YannickJadoul commented Oct 15, 2020

At the same time, it's still quite late to add, though? :-/ @henryiii?

@YannickJadoul
Copy link
Collaborator

Also, what if the new class has exactly the same name as the old one?

@bstaletic
Copy link
Collaborator

Also, also, the changes to the hash function are really wrong. You don't want to combine hashes with just an xor.

@henryiii
Copy link
Collaborator

We can work on it for 2.7 - I think it's too late for 2.6 for anything with a chance of breaking.

@EricCousineau-TRI
Copy link
Collaborator

Sounds good! We'll address this for release 2.7.0, not 2.6.0 - thanks!

@EricCousineau-TRI
Copy link
Collaborator

Per Slack convo w/ Betsy and @jamiesnape, I will re-open this as author (but maintain Betsy's authorship + traceability). Then I'll update it for latest master, and see if we can sneak it into future releases.

@EricCousineau-TRI
Copy link
Collaborator

Closed in lieu of superseding PR, #2772

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.

5 participants