Skip to content

Conversation

@cnaples79
Copy link
Contributor

@cnaples79 cnaples79 commented Sep 21, 2025

Summary

  • Import tf_agents.policies.policy_saver as tf_policy_saver
  • Update usages and type hints to use tf_policy_saver.PolicySaver

This clarifies that MLGOPolicySaver is our wrapper, and PolicySaver refers to TF-Agents.

Refs #309

@mtrofin
Copy link
Collaborator

mtrofin commented Sep 22, 2025

Thanks - can you please rename the title with the prefix [nfc] (a'la LLVM. Signals to reviewers that "no tests needed")

Could you also reduce the message to e.g. what you have in the commit (pasted below for clarity)? Was the current commit message AI-generated? It's missing text, e.g. "Import TF‑Agents policy saver as .". Anyway, no need for overly-formatted stuff, the simpler message is perfect!

- Import tf_agents.policies.policy_saver as tf_policy_saver
- Update usages and type hints to use tf_policy_saver.PolicySaver

This clarifies that MLGOPolicySaver is our wrapper, and PolicySaver refers to TF-Agents.

Thanks!

@cnaples79 cnaples79 force-pushed the feature/alias-tf-policy-saver-309 branch from 0699fc3 to add937a Compare September 23, 2025 00:03
@cnaples79 cnaples79 changed the title readability: alias tf_agents.policy_saver to avoid name confusion with local module [nfc] alias tf_agents.policies.policy_saver as tf_policy_saver; update references Sep 23, 2025
- Import tf_agents.policies.policy_saver as tf_policy_saver
- Update usages and type hints to use tf_policy_saver.PolicySaver

This clarifies that MLGOPolicySaver is our wrapper, and PolicySaver refers to TF-Agents.

No functional change. Refs google#309
@cnaples79 cnaples79 force-pushed the feature/alias-tf-policy-saver-309 branch from add937a to bc979d4 Compare September 23, 2025 00:10
@cnaples79 cnaples79 changed the title [nfc] alias tf_agents.policies.policy_saver as tf_policy_saver; update references [nfc] alias tf_agents.policies.policy_saver as tf_policy_saver Sep 23, 2025
@cnaples79
Copy link
Contributor Author

@mtrofin should be fixed! Let me know if I need to make any other changes.

@mtrofin
Copy link
Collaborator

mtrofin commented Sep 23, 2025

@mtrofin should be fixed! Let me know if I need to make any other changes.

The commit message?

@cnaples79
Copy link
Contributor Author

@mtrofin I updated both the PR body and the commit message. Is it still not showing as changed?

@mtrofin mtrofin merged commit d200783 into google:main Sep 23, 2025
12 checks passed
@mtrofin
Copy link
Collaborator

mtrofin commented Sep 23, 2025

@mtrofin I updated both the PR body and the commit message. Is it still not showing as changed?

Does now - thank you!

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