Skip to content

Python: Use only TApiNode in API::Impl#5770

Merged
yoff merged 1 commit into
github:mainfrom
tausbn:python-small-api-graph-fix
Apr 27, 2021
Merged

Python: Use only TApiNode in API::Impl#5770
yoff merged 1 commit into
github:mainfrom
tausbn:python-small-api-graph-fix

Conversation

@tausbn
Copy link
Copy Markdown
Contributor

@tausbn tausbn commented Apr 26, 2021

This ensures that changes to API::Node do not invalidate the cached
module Impl. At present, I don't expect this to have any effect (as
the Node class is also fairly static, though not explicitly cached),
but I can imagine us making some of the Node methods have
user-extensible behaviour, in which case we definitely do not want this
to result in reevaluation of API::Impl.

Doesn't change behaviour, so no change note needed.

This ensures that changes to `API::Node` does not invalidate the cached
`module Impl`. At present, I don't expect this to have any effect (as
the `Node` class is also fairly static, though not explicitly cached),
but I can imagine us making some of the `Node` methods have
user-extensible behaviour, in which case we definitely do not want this
to result in reevaluation of `API::Impl`.
@tausbn tausbn added the no-change-note-required This PR does not need a change note label Apr 26, 2021
@tausbn tausbn requested a review from a team as a code owner April 26, 2021 13:17
@tausbn
Copy link
Copy Markdown
Contributor Author

tausbn commented Apr 26, 2021

Not much change in performance: https://github.com/dsp-testing/tausbn-dca/issues/20
Though to be honest, not much change was expected, either. 🙂

Copy link
Copy Markdown
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

LGTM

@yoff yoff merged commit 0509a12 into github:main Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants