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

Require Python 3.9+, test for 3.9 and 3.11 #1924

Closed
wants to merge 2 commits into from
Closed

Conversation

esantorella
Copy link
Member

@esantorella esantorella commented Jul 10, 2023

Motivation

Python 3.11 was released over 8 months ago, so we should be testing for it. We can also drop support for Python 3.8, which was supplanted by 3.9 nearly 3 years ago.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Manually running nightly cron: https://github.com/pytorch/botorch/actions/runs/5508340274

@esantorella esantorella self-assigned this Jul 10, 2023
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jul 10, 2023
@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Merging #1924 (77d5b0c) into main (71fd34e) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

❗ Current head 77d5b0c differs from pull request most recent head bb9c29b. Consider uploading reports for the commit bb9c29b to get more accurate results

@@           Coverage Diff           @@
##             main    #1924   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files         177      177           
  Lines       15683    15683           
=======================================
  Hits        15675    15675           
  Misses          8        8           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Balandat
Copy link
Contributor

Interesting, looks like py 3.11 is stricter on the dataclasses, I guess this will require a small patch.

facebook-github-bot pushed a commit that referenced this pull request Jul 10, 2023
…1927)

Summary:
## Motivation

See error here: https://github.com/pytorch/botorch/actions/runs/5508336054/jobs/10039481686?pr=1924
* Tested with a tensor instead of a `DenseContainer` since we needed something non-mutable
* Added a test that the tensor is properly cast to a `DenseContainer`
* made code more readable (for me anyway)

### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)?

Yes

Pull Request resolved: #1927

Test Plan:
Units

## Related PRs

Unblocks #1924

Reviewed By: saitcakmak

Differential Revision: D47341881

Pulled By: esantorella

fbshipit-source-id: 64c497504bffde8cc227dff8eaaa85e17aa36b95
@facebook-github-bot
Copy link
Contributor

@esantorella has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@saitcakmak saitcakmak left a comment

Choose a reason for hiding this comment

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

I think Python 3.8 was the latest version when I started using BoTorch. Python evolves fast!

@facebook-github-bot
Copy link
Contributor

@esantorella merged this pull request in 8c763b3.

facebook-github-bot pushed a commit to facebook/Ax that referenced this pull request Jul 26, 2023
Summary:
Follow up to pytorch/botorch#1924 to avoid CI failures.

NOTE: We cannot support 3.11 currently due to a failure in torchx tests. See pytorch/torchx#744

Pull Request resolved: #1744

Reviewed By: esantorella

Differential Revision: D47780385

Pulled By: saitcakmak

fbshipit-source-id: af69631befbdb3035f1a969c03d73defb070e19e
@Balandat Balandat deleted the python_version_39 branch August 1, 2023 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants