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

MNT: pass callbacks directly to _RE.__call__ #308

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tacaswell
Copy link
Collaborator

Description

The second positional argument to RE() is subscriptions to processed for that invocation. This avoids adding one more layer to the callstack and a possible confusion with plans that do not set their own name being reported as having the 'plan_name' of 'subs_wrapper' in the start document.

Motivation and Context

This caused some confusion at HEX. The ultimate fix is that plans that generate start documents should insert their own name into the meta-data of the open_run plan so that when used from inside of a bigger plan the name is stable. This change will not fix the ultimate problem, but it will change what the user sees to be the name of the plan they wrote so it will be clearer what is going on and what they have to do to fix it.

Summary of Changes for Release Notes

How Has This Been Tested?

Expect all tests to pass after this change and there to be no meaningful user-facing change.

The second positional argument to RE() is subscriptions to processed for that
invocation.  This avoids adding one more layer to the callstack and a possible
confusion with plans that do not set their own name being reported as having
the `'plan_name'` of `'subs_wrapper'` in the start document.
@mrakitin
Copy link
Member

Is it this warning which causes the documentation to fail?

WARNING: Calling get_html_theme_path is deprecated. If you are calling it to define html_theme_path, you are safe to remove that code.

@mrakitin
Copy link
Member

We should test it in the lab before merging.
cc @jwlodek

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.

2 participants