[MLA-1731] update pip instructions (#5074)

Author: Chris Elion

docs/Installation-Anaconda-Windows.md

@@ -151,7 +151,7 @@ config files in this directory when running `mlagents-learn`. Make sure you are
 connected to the Internet and then type in the Anaconda Prompt:
 
 ```console
-pip install mlagents
+python -m pip install mlagents==0.24.1
 ```
 
 This will complete the installation of all the required Python packages to run
@@ -162,7 +162,7 @@ pip will get stuck when trying to read the cache of the package. If you see
 this, you can try:
 
 ```console
-pip install mlagents --no-cache-dir
+python -m pip install mlagents==0.24.1 --no-cache-dir
 ```
 
 This `--no-cache-dir` tells the pip to disable the cache.

docs/Installation.md

@@ -154,7 +154,7 @@ To install the `mlagents` Python package, activate your virtual environment and
 run from the command line:
 
 ```sh
-pip3 install mlagents
+python -m pip install mlagents==0.24.1
 ```
 
 Note that this will install `mlagents` from PyPi, _not_ from the cloned

docs/Training-on-Microsoft-Azure.md

@@ -33,7 +33,7 @@ view the documentation for doing so [here](#custom-instances).
    instance, and set it as the working directory.
 2. Install the required packages:
    Torch: `pip3 install torch==1.7.0 -f https://download.pytorch.org/whl/torch_stable.html` and
-   MLAgents: `pip3 install mlagents`
+   MLAgents: `python -m pip install mlagents==0.24.1`
 
 ## Testing
 

ml-agents-envs/README.md

@@ -13,7 +13,7 @@ communication.
 Install the `mlagents_envs` package with:
 
 ```sh
-pip3 install mlagents_envs
+python -m pip install mlagents_envs==0.24.1
 ```
 
 ## Usage & More Information

ml-agents/README.md

@@ -16,7 +16,7 @@ package.
 Install the `mlagents` package with:
 
 ```sh
-pip3 install mlagents
+python -m pip install mlagents==0.24.1
 ```
 
 ## Usage & More Information

utils/validate_release_links.py

@@ -9,11 +9,17 @@
 from typing import List, Optional, Pattern
 
 RELEASE_PATTERN = re.compile(r"release_[0-9]+(_docs)*")
+# This matches the various ways to invoke pip: "pip", "pip3", "python -m pip"
+# It matches "mlagents" and "mlagents_envs", accessible as group "package"
+# and optionally matches the version, e.g. "==1.2.3"
+PIP_INSTALL_PATTERN = re.compile(
+    r"(python -m )?pip3* install (?P<package>mlagents(_envs)?)(==[0-9]\.[0-9]\.[0-9](\.dev[0-9]+)?)?"
+)
 TRAINER_INIT_FILE = "ml-agents/mlagents/trainers/__init__.py"
 
 MATCH_ANY = re.compile(r"(?s).*")
 # Filename -> regex list to allow specific lines.
-# To allow everything in the file, use None for the value
+# To allow everything in the file (effectively skipping it), use MATCH_ANY for the value
 ALLOW_LIST = {
     # Previous release table
     "README.md": re.compile(r"\*\*(Verified Package ([0-9]\.?)*|Release [0-9]+)\*\*"),
@@ -24,27 +30,61 @@
 }
 
 
-def test_pattern():
+def test_release_pattern():
     # Just some sanity check that the regex works as expected.
-    assert RELEASE_PATTERN.search(
-        "https://github.com/Unity-Technologies/ml-agents/blob/release_4_docs/Food.md"
-    )
-    assert RELEASE_PATTERN.search(
-        "https://github.com/Unity-Technologies/ml-agents/blob/release_4/Foo.md"
-    )
-    assert RELEASE_PATTERN.search(
-        "git clone --branch release_4 https://github.com/Unity-Technologies/ml-agents.git"
-    )
-    assert RELEASE_PATTERN.search(
-        "https://github.com/Unity-Technologies/ml-agents/blob/release_123_docs/Foo.md"
-    )
-    assert RELEASE_PATTERN.search(
-        "https://github.com/Unity-Technologies/ml-agents/blob/release_123/Foo.md"
-    )
-    assert not RELEASE_PATTERN.search(
-        "https://github.com/Unity-Technologies/ml-agents/blob/latest_release/docs/Foo.md"
+    for s, expected in [
+        (
+            "https://github.com/Unity-Technologies/ml-agents/blob/release_4_docs/Food.md",
+            True,
+        ),
+        ("https://github.com/Unity-Technologies/ml-agents/blob/release_4/Foo.md", True),
+        (
+            "git clone --branch release_4 https://github.com/Unity-Technologies/ml-agents.git",
+            True,
+        ),
+        (
+            "https://github.com/Unity-Technologies/ml-agents/blob/release_123_docs/Foo.md",
+            True,
+        ),
+        (
+            "https://github.com/Unity-Technologies/ml-agents/blob/release_123/Foo.md",
+            True,
+        ),
+        (
+            "https://github.com/Unity-Technologies/ml-agents/blob/latest_release/docs/Foo.md",
+            False,
+        ),
+    ]:
+        assert bool(RELEASE_PATTERN.search(s)) is expected
+
+    print("release tests OK!")
+
+
+def test_pip_pattern():
+    # Just some sanity check that the regex works as expected.
+    for s, expected in [
+        ("pip install mlagents", True),
+        ("pip3 install mlagents", True),
+        ("python -m pip install mlagents", True),
+        ("python -m pip install mlagents==1.2.3", True),
+        ("python -m pip install mlagents_envs==1.2.3", True),
+    ]:
+        assert bool(PIP_INSTALL_PATTERN.search(s)) is expected
+
+    sub_expected = "Try running rm -rf / to install"
+    assert sub_expected == PIP_INSTALL_PATTERN.sub(
+        "rm -rf /", "Try running python -m pip install mlagents==1.2.3 to install"
     )
-    print("tests OK!")
+
+    print("pip tests OK!")
+
+
+def update_pip_install_line(line, package_verion):
+    match = PIP_INSTALL_PATTERN.search(line)
+    package_name = match.group("package")
+    replacement_version = f"python -m pip install {package_name}=={package_verion}"
+    updated = PIP_INSTALL_PATTERN.sub(replacement_version, line)
+    return updated
 
 
 def git_ls_files() -> List[str]:
@@ -74,8 +114,28 @@ def get_release_tag() -> Optional[str]:
     raise RuntimeError("Can't determine release tag")
 
 
+def get_python_package_version() -> str:
+    """
+    Returns the mlagents python package.
+    :return:
+    """
+    with open(TRAINER_INIT_FILE) as f:
+        for line in f:
+            if "__version__" in line:
+                lhs, equals_string, rhs = line.strip().partition(" = ")
+                # Evaluate the right hand side of the expression
+                return ast.literal_eval(rhs)
+    # If we couldn't find the release tag, raise an exception
+    # (since we can't return None here)
+    raise RuntimeError("Can't determine python package version")
+
+
 def check_file(
-    filename: str, global_allow_pattern: Pattern, release_tag: str
+    filename: str,
+    release_tag_pattern: Pattern,
+    release_tag: str,
+    pip_allow_pattern: Pattern,
+    package_version: str,
 ) -> List[str]:
     """
     Validate a single file and return any offending lines.
@@ -90,21 +150,37 @@ def check_file(
             allow_list_pattern = ALLOW_LIST.get(filename, None)
             with open(filename) as f:
                 for line in f:
-                    keep_line = True
-                    keep_line = not RELEASE_PATTERN.search(line)
-                    keep_line |= global_allow_pattern.search(line) is not None
-                    keep_line |= (
-                        allow_list_pattern is not None
+                    # Does it contain anything of the form release_123
+                    has_release_pattern = RELEASE_PATTERN.search(line) is not None
+                    # Does it contain this particular release, e.g. release_42 or release_42_docs
+                    has_release_tag_pattern = (
+                        release_tag_pattern.search(line) is not None
+                    )
+                    # Does it contain the allow list pattern for the file (if there is one)
+                    has_allow_list_pattern = (
+                        allow_list_pattern
                         and allow_list_pattern.search(line) is not None
                     )
 
-                    if keep_line:
+                    pip_install_ok = (
+                        has_allow_list_pattern
+                        or PIP_INSTALL_PATTERN.search(line) is None
+                        or pip_allow_pattern.search(line) is not None
+                    )
+
+                    release_tag_ok = (
+                        not has_release_pattern
+                        or has_release_tag_pattern
+                        or has_allow_list_pattern
+                    )
+
+                    if release_tag_ok and pip_install_ok:
                         new_file.write(line)
                     else:
                         bad_lines.append(f"{filename}: {line}")
-                        new_file.write(
-                            re.sub(r"release_[0-9]+", fr"{release_tag}", line)
-                        )
+                        new_line = re.sub(r"release_[0-9]+", fr"{release_tag}", line)
+                        new_line = update_pip_install_line(new_line, package_version)
+                        new_file.write(new_line)
         if bad_lines:
             if os.path.exists(filename):
                 os.remove(filename)
@@ -113,17 +189,28 @@ def check_file(
     return bad_lines
 
 
-def check_all_files(allow_pattern: Pattern, release_tag: str) -> List[str]:
+def check_all_files(
+    release_allow_pattern: Pattern,
+    release_tag: str,
+    pip_allow_pattern: Pattern,
+    package_version: str,
+) -> List[str]:
     """
     Validate all files tracked by git.
-    :param allow_pattern:
+    :param release_allow_pattern:
     """
     bad_lines = []
     file_types = {".py", ".md", ".cs"}
     for file_name in git_ls_files():
         if "localized" in file_name or os.path.splitext(file_name)[1] not in file_types:
             continue
-        bad_lines += check_file(file_name, allow_pattern, release_tag)
+        bad_lines += check_file(
+            file_name,
+            release_allow_pattern,
+            release_tag,
+            pip_allow_pattern,
+            package_version,
+        )
     return bad_lines
 
 
@@ -133,9 +220,16 @@ def main():
         print("Release tag is None, exiting")
         sys.exit(0)
 
+    package_version = get_python_package_version()
     print(f"Release tag: {release_tag}")
-    allow_pattern = re.compile(f"{release_tag}(_docs)*")
-    bad_lines = check_all_files(allow_pattern, release_tag)
+    print(f"Python package version: {package_version}")
+    release_allow_pattern = re.compile(f"{release_tag}(_docs)?")
+    pip_allow_pattern = re.compile(
+        f"python -m pip install mlagents(_envs)?=={package_version}"
+    )
+    bad_lines = check_all_files(
+        release_allow_pattern, release_tag, pip_allow_pattern, package_version
+    )
     if bad_lines:
         for line in bad_lines:
             print(line)
@@ -151,5 +245,6 @@ def main():
 
 if __name__ == "__main__":
     if "--test" in sys.argv:
-        test_pattern()
+        test_release_pattern()
+        test_pip_pattern()
     main()