Skip to content

Conversation

@aslonnie
Copy link
Collaborator

stop running on python 3.9 any more

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request migrates most of the ML tests in the Buildkite CI pipeline from Python 3.9 to 3.10. The changes involve updating Python version matrices, hardcoding Python 3.10 in various test steps, and adjusting build dependencies. Overall, the changes are consistent with the goal of the PR. I've found one critical typo in a dependency name that needs to be corrected to prevent CI failures.

--parallelism-per-worker 3
--skip-ray-installation
depends_on: [ "mlbuild", "forge" ]
depends_on: [ "mlbuild-multiply", "forge" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

There seems to be a typo here. mlbuild-multiply should probably be mlbuild-multipy to be consistent with other changes in this file. This will likely break this CI step.

    depends_on: [ "mlbuild-multipy", "forge" ]

--parallelism-per-worker 3
--skip-ray-installation
depends_on: [ "mlbuild", "forge" ]
depends_on: [ "mlbuild-multiply", "forge" ]
Copy link

Choose a reason for hiding this comment

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

Bug: Typo in dependency name

The depends_on field references mlbuild-multiply instead of mlbuild-multipy. This typo causes the doc tests job to depend on a non-existent build step, which will prevent the pipeline from running correctly. The correct build name is mlbuild-multipy as defined earlier in the file and used consistently in all other test jobs.

Fix in Cursor Fix in Web

@aslonnie aslonnie force-pushed the lonnie-251121-mlpy310 branch from ad575cb to 7272000 Compare November 22, 2025 00:50
@aslonnie aslonnie added the go add ONLY when ready to merge, run all tests label Nov 22, 2025
@ray-gardener ray-gardener bot added the train Ray Train Related Issue label Nov 22, 2025
stop running on python 3.9 any more

Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
@aslonnie aslonnie force-pushed the lonnie-251121-mlpy310 branch from 622182b to e093586 Compare November 22, 2025 04:26
@aslonnie
Copy link
Collaborator Author

@matthewdeng @justinvyu do you know how to fix the following error when upgraded to python 3.10?


[2025-11-22T04:03:29Z] set_search_properties_func = <bound method SearchSpaceTest.testSetSearchPropertiesBackwardsCompatibility.<locals>.MySearcher.set_search_properties ...est_sample.SearchSpaceTest.testSetSearchPropertiesBackwardsCompatibility.<locals>.MySearcher object at 0x7f48d91577f0>>
--
[2025-11-22T04:03:29Z] metric = None, mode = None, config = {'a': 1}
[2025-11-22T04:03:29Z] spec = {'num_samples': 1, 'stop': {}, 'time_budget_s': None}
[2025-11-22T04:03:29Z]
[2025-11-22T04:03:29Z]     def _set_search_properties_backwards_compatible(
[2025-11-22T04:03:29Z]         set_search_properties_func,
[2025-11-22T04:03:29Z]         metric: Optional[str],
[2025-11-22T04:03:29Z]         mode: Optional[str],
[2025-11-22T04:03:29Z]         config: Dict,
[2025-11-22T04:03:29Z]         **spec
[2025-11-22T04:03:29Z]     ) -> bool:
[2025-11-22T04:03:29Z]         """Wraps around set_search_properties() so that it is backward compatible.
[2025-11-22T04:03:29Z]
[2025-11-22T04:03:29Z]         Also outputs a warning to encourage custom searchers to be updated.
[2025-11-22T04:03:29Z]         """
[2025-11-22T04:03:29Z]         try:
[2025-11-22T04:03:29Z] >           return set_search_properties_func(metric, mode, config, **spec)
[2025-11-22T04:03:29Z] E           TypeError: SearchSpaceTest.testSetSearchPropertiesBackwardsCompatibility.<locals>.MySearcher.set_search_properties() got an unexpected keyword argument 'stop'
[2025-11-22T04:03:29Z]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests train Ray Train Related Issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants