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

Return parameter values from the orderly.parameters call. #49

Merged
merged 2 commits into from
May 10, 2024

Conversation

plietar
Copy link
Member

@plietar plietar commented May 3, 2024

Previously we would inject parameter values as global variables before the script starts executing. While this follows the behaviour of orderly2, it confuses a lot of Python tooling, including linters and type checkers. It also did not support executing orderly scripts outside of a packet context (eg. interactively).

The orderly.parameters function now returns the parameter values as a dictionary, which we can get from the global context. When running outside of a packet context, it returns the parameters' default values.

Previously we would inject parameter values as global variables before
the script starts executing. While this follows the behaviour of
orderly2, it confuses a lot of Python tooling, including linters and
type checkers. It also did not support executing orderly scripts outside
of a packet context (eg. interactively).

The `orderly.parameters` function now returns the parameter values as a
dictionary, which we can get from the global context. When running
outside of a packet context, it returns the parameters' default values.
@plietar plietar force-pushed the mrc-5255-return-parameters branch from fd63e8c to 927fafa Compare May 3, 2024 13:22
@plietar plietar requested a review from richfitz May 3, 2024 15:27
with open("result.txt", "w") as f:
f.write(f"a: {a}\nb: {b}\n") # type: ignore
f.write(f"a: {parameters['a']}\nb: {parameters['b']}\n")
Copy link
Member

Choose a reason for hiding this comment

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

Add some sugar to allow parameters.a perhaps?

That (or anything slightly more sophisticated than just using a dict for the storage) would also allow nicer error messages on near miss parameter usage, so that parameters['whatever'] or parameters.whatever might return an error that lists known parameters

Copy link
Member Author

Choose a reason for hiding this comment

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

I've replaced it with a SimpleNamespace subclass, which has .foo lookup. The subclass is empty for now (other than the inherited init/eq/repr methods), but we can extend it in the future.

@plietar plietar requested a review from richfitz May 8, 2024 15:32
@plietar plietar merged commit 03cbad5 into main May 10, 2024
7 checks passed
@plietar plietar deleted the mrc-5255-return-parameters branch May 10, 2024 13:11
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.

None yet

2 participants