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

chore(deps): Bump cda-tum/mqt-workflows from 1.1.4 to 1.1.5 in the github-actions group #57

Merged
merged 6 commits into from
Jul 29, 2024

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jul 26, 2024

Bumps the github-actions group with 1 update: cda-tum/mqt-workflows.

Updates cda-tum/mqt-workflows from 1.1.4 to 1.1.5

Release notes

Sourced from cda-tum/mqt-workflows's releases.

MQT Reusable Workflows 1.1.5 Release

👀 What Changed

This patch release adds more flexibility to the cpp-linter workflow.

🚀 Features and Enhancements

🤖 CI

⬆️ Dependencies

Full Changelog: cda-tum/mqt-workflows@v1.1.4...v1.1.5

Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Dependabot will merge this PR once CI passes on it, as requested by @burgholzer.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore <dependency name> major version will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself)
  • @dependabot ignore <dependency name> minor version will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself)
  • @dependabot ignore <dependency name> will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself)
  • @dependabot unignore <dependency name> will remove all of the ignore conditions of the specified dependency
  • @dependabot unignore <dependency name> <ignore condition> will remove the ignore condition of the specified dependency and ignore conditions

Bumps the github-actions group with 1 update: [cda-tum/mqt-workflows](https://github.com/cda-tum/mqt-workflows).


Updates `cda-tum/mqt-workflows` from 1.1.4 to 1.1.5
- [Release notes](https://github.com/cda-tum/mqt-workflows/releases)
- [Commits](cda-tum/mqt-workflows@v1.1.4...v1.1.5)

---
updated-dependencies:
- dependency-name: cda-tum/mqt-workflows
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: github-actions
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot added dependencies Pull requests that update a dependency file github_actions Pull requests that update GitHub Actions code labels Jul 26, 2024
@burgholzer
Copy link
Member

@DRovara Some python dependency update must have broken some things here. I suspect it's a sympy update, but haven't had time to look deeper into it.
Maybe you have some time to look into this further 😌

@DRovara
Copy link
Collaborator

DRovara commented Jul 26, 2024

@DRovara Some python dependency update must have broken some things here. I suspect it's a sympy update, but haven't had time to look deeper into it. Maybe you have some time to look into this further 😌

It seems the test cases fail because of the docplex and numpy dependencies. With numpy 2.0, the type float_ is no longer supported, but docplex uses it, which causes all tests to fail. Maybe the straightforward solution would be to prohibit numpy >= 2.0 for now, until docplex fixes it?

The linter issues seem to be a different problem, though. Still looking into that.

@burgholzer
Copy link
Member

@DRovara Some python dependency update must have broken some things here. I suspect it's a sympy update, but haven't had time to look deeper into it. Maybe you have some time to look into this further 😌

It seems the test cases fail because of the docplex and numpy dependencies. With numpy 2.0, the type float_ is no longer supported, but docplex uses it, which causes all tests to fail. Maybe the straightforward solution would be to prohibit numpy >= 2.0 for now, until docplex fixes it?

The linter issues seem to be a different problem, though. Still looking into that.

Yeah. Pinning numpy<2.0 could be a suitable workaround for now. Although I'd really like to keep this as shortlived as possible. It is not good if the whole ecosystem starts to put upper caps on numpy and prevents its adoption.

You can take inspiration from https://github.com/cda-tum/mqt-qecc/blob/6e581cf20e0a4b31a79078a97669290587cff5c5/pyproject.toml#L43-L44

If there is no issue over at the docplex repository for numpy 2.0 compatibility yet, I would highly encourage creating one.

The linter issues mostly seem due to sympy. 1.3.0 was released three weeks ago. The release notes contain some breaking changes that are probably at fault here: https://github.com/sympy/sympy/wiki/release-notes-for-1.13.0

@DRovara
Copy link
Collaborator

DRovara commented Jul 26, 2024

I have now bitten the bullet and wrote type stubs for all sympy features we use. This should hopefully resolve the linting issues. I still don't know why the errors suddenly came up now, but checking the update notes of the latest sympy version, none of the changes mentioned seem to be related to this.

In any case, writing type stubs for the sympy features we use was an open task anyways, and doing that allowed us to get rid of a lot of lining ignore rules.

burgholzer
burgholzer previously approved these changes Jul 26, 2024
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Nice! This makes the code so much nicer to look at ✨
Assuming that all tests pass, this is good to go I believe.

@dependabot merge

pyproject.toml Outdated Show resolved Hide resolved
@burgholzer
Copy link
Member

I still don't know why the errors suddenly came up now, but checking the update notes of the latest sympy version, none of the changes mentioned seem to be related to this.

I think the critical one might have been

BREAKING CHANGE: Float and Integer/Rational no longer compare equal with a == b. From now on Float(2.0) != Integer(2). Previously expressions involving Float would compare unequal e.g. x2.0 != x2 but an individual Float would compare equal to an Integer. In SymPy 1.7 a Float will always compare unequal to an Integer even if they have the same "value". Use sympy.numbers.int_valued(number) to test if a number is a concrete number with no decimal part. (#25614 by @smichr)

Copy link
Contributor Author

dependabot bot commented on behalf of github Jul 26, 2024

One of your CI runs failed on this pull request, so Dependabot won't merge it.

Dependabot will still automatically merge this pull request if you amend it and your tests pass.

@burgholzer
Copy link
Member

I think the test failures here might actually be due to the change I pasted above.

@DRovara
Copy link
Collaborator

DRovara commented Jul 26, 2024

I still don't know why the errors suddenly came up now, but checking the update notes of the latest sympy version, none of the changes mentioned seem to be related to this.

I think the critical one might have been

BREAKING CHANGE: Float and Integer/Rational no longer compare equal with a == b. From now on Float(2.0) != Integer(2). Previously expressions involving Float would compare unequal e.g. x_2.0 != x_2 but an individual Float would compare equal to an Integer. In SymPy 1.7 a Float will always compare unequal to an Integer even if they have the same "value". Use sympy.numbers.int_valued(number) to test if a number is a concrete number with no decimal part. (#25614 by @smichr)

The problems were all linter errors. In particular, mypy suddenly started giving

error: Skipping analyzing "sympy": module is installed, but missing library stubs or py.typed marker  [import-untyped]
    import sympy as sp

And as sympy was now not recognised by mypy anymore, all calls related to it (that previously had ignore rules for mypy due to missing type information) caused linter errors as the ignore rules were now unused.

I'm not sure if py.typed was removed from sympy in the newest version, or if mypy just got stricter, but sympy has never had type hints anyways, so that's where my confusion comes from.

The breaking change you mention, while it may come up during execution, does not seem like it should cause any of these problems (unless I am overlooking something). But as you say, it might be the cause of the remaining test errors. I'll check that out.

@burgholzer
Copy link
Member

I still don't know why the errors suddenly came up now, but checking the update notes of the latest sympy version, none of the changes mentioned seem to be related to this.

I think the critical one might have been

BREAKING CHANGE: Float and Integer/Rational no longer compare equal with a == b. From now on Float(2.0) != Integer(2). Previously expressions involving Float would compare unequal e.g. x_2.0 != x_2 but an individual Float would compare equal to an Integer. In SymPy 1.7 a Float will always compare unequal to an Integer even if they have the same "value". Use sympy.numbers.int_valued(number) to test if a number is a concrete number with no decimal part. (#25614 by @smichr)

The problems were all linter errors. In particular, mypy suddenly started giving

error: Skipping analyzing "sympy": module is installed, but missing library stubs or py.typed marker  [import-untyped]
    import sympy as sp

And as sympy was now not recognised by mypy anymore, all calls related to it (that previously had ignore rules for mypy due to missing type information) caused linter errors as the ignore rules were now unused.

I'm not sure if py.typed was removed from sympy in the newest version, or if mypy just got stricter, but sympy has never had type hints anyways, so that's where my confusion comes from.

The breaking change you mention, while it may come up during execution, does not seem like it should cause any of these problems (unless I am overlooking something). But as you say, it might be the cause of the remaining test errors. I'll check that out.

My guess is simply based on the test failures here: https://github.com/cda-tum/mqt-qubomaker/actions/runs/10109367250/job/27957235567?pr=57#step:10:3944

These are independent from the linter failures earlier that you nicely fixed already 😌 (those probably came up due to the mypy 1.11 update in pre-commit).

Co-authored-by: Lukas Burgholzer <[email protected]>
Signed-off-by: Damian Rovara <[email protected]>
@DRovara
Copy link
Collaborator

DRovara commented Jul 26, 2024

My guess is simply based on the test failures here: https://github.com/cda-tum/mqt-qubomaker/actions/runs/10109367250/job/27957235567?pr=57#step:10:3944

These are independent from the linter failures earlier that you nicely fixed already 😌 (those probably came up due to the mypy 1.11 update in pre-commit).

Sorry, I thought you were talking about the linter issues, too. At the time, I didn't realise some tests were still failing. After local testing, it seems your hunch was correct: The errors were caused by int vs float comparisons in the tests. This should now be fixed.

@burgholzer
Copy link
Member

My guess is simply based on the test failures here: https://github.com/cda-tum/mqt-qubomaker/actions/runs/10109367250/job/27957235567?pr=57#step:10:3944
These are independent from the linter failures earlier that you nicely fixed already 😌 (those probably came up due to the mypy 1.11 update in pre-commit).

Sorry, I thought you were talking about the linter issues, too. At the time, I didn't realise some tests were still failing. After local testing, it seems your hunch was correct: The errors were caused by int vs float comparisons in the tests. This should now be fixed.

No worries and thanks for fixing the underlying issues! Dependabot should merge the PR if all checks pass 😌

Copy link

codecov bot commented Jul 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.5%. Comparing base (81ba42f) to head (c57aa91).
Report is 49 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main     #57     +/-   ##
=======================================
+ Coverage   91.8%   93.5%   +1.7%     
=======================================
  Files          8       8             
  Lines        912     905      -7     
=======================================
+ Hits         838     847      +9     
+ Misses        74      58     -16     
Flag Coverage Δ
python 93.5% <100.0%> (+1.7%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DRovara DRovara merged commit 68dd1a7 into main Jul 29, 2024
30 checks passed
@DRovara DRovara deleted the dependabot/github_actions/github-actions-a880fba8bf branch July 29, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file github_actions Pull requests that update GitHub Actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants