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

remove _T dependency #3903

Merged
merged 5 commits into from
Jan 7, 2025
Merged

remove _T dependency #3903

merged 5 commits into from
Jan 7, 2025

Conversation

MilesHolland
Copy link
Member

Marshmallow 3.24 dropped today, and removed the _T import, which broke PF. This pr fixes that hopefully. https://pypi.org/project/marshmallow/#history

kdestin
kdestin previously approved these changes Jan 6, 2025
@kdestin
Copy link
Contributor

kdestin commented Jan 6, 2025

Copy link

github-actions bot commented Jan 6, 2025

promptflow SDK CLI Azure E2E Test Result jan25/pf/bugfix/no-marshmallow-t

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 4b80f0c.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 6, 2025

SDK CLI Global Config Test Result jan25/pf/bugfix/no-marshmallow-t

6 tests   6 ✅  1m 20s ⏱️
1 suites  0 💤
1 files    0 ❌

Results for commit 4b80f0c.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 6, 2025

Executor Unit Test Result jan25/pf/bugfix/no-marshmallow-t

798 tests   798 ✅  3m 43s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 4b80f0c.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 6, 2025

Executor E2E Test Result jan25/pf/bugfix/no-marshmallow-t

245 tests   239 ✅  5m 11s ⏱️
  1 suites    6 💤
  1 files      0 ❌

Results for commit 4b80f0c.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 6, 2025

SDK CLI Test Result jan25/pf/bugfix/no-marshmallow-t

    3 files      3 suites   48m 5s ⏱️
  790 tests   766 ✅ 23 💤 1 ❌
2 370 runs  2 298 ✅ 69 💤 3 ❌

For more details on these failures, see this check.

Results for commit 4b80f0c.

♻️ This comment has been updated with latest results.

@MilesHolland
Copy link
Member Author

/check-enforcer override

@ninghu ninghu merged commit a981d6a into main Jan 7, 2025
23 of 38 checks passed
@ninghu ninghu deleted the jan25/pf/bugfix/no-marshmallow-t branch January 7, 2025 17:47
@sloria
Copy link

sloria commented Jan 21, 2025

sorry for the breakage 😬 . this is the correct fix. just a heads up: the TypeVars in marshmallow (and any library) should not be imported in userland

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

Successfully merging this pull request may close these issues.

5 participants