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

Fix pipeline #164

Merged
merged 4 commits into from
Apr 10, 2024
Merged

Fix pipeline #164

merged 4 commits into from
Apr 10, 2024

Conversation

BenPortner
Copy link
Member

No description provided.

@BenPortner
Copy link
Member Author

@cmutel apparently, bw2data is currently broken when installed from pip due to outdated dependencies. Could you update packages on pypi to match the status of conda?

@BenPortner BenPortner requested a review from cmutel March 30, 2024 18:44
@BenPortner
Copy link
Member Author

@tngTUDOR @michaelweinold anything you can do?

@michaelweinold
Copy link
Contributor

This is likely caused by a change in the pytest functionality (cf. related question on SO). Should be an easy fix...

@BenPortner
Copy link
Member Author

BenPortner commented Apr 5, 2024

@michaelweinold note that the tests work when installing with conda. I believe the packages on pypi are outdated

@michaelweinold
Copy link
Contributor

@BenPortner, which packaged do you think are missing in PyPi?

From what I can see, the pip tests use pytest~7.X.X, which raises a warning:

  /home/vsts/work/1/s/bw2data/tests.py:50: PytestRemovedIn8Warning: Passing None has been deprecated.

but still lets those tests using pytest.warns(None) pass.

The Conda tests use pytest~8.X.X, which results in test failures with

TypeError: exceptions must be derived from Warning, not <class 'NoneType'>

The changelog of pytest is a little confusing as to with which pytest version exactly the warns(None) was deprecated - but from the above it seems that only in 8.X.X this was finalized.

I won't have time to check what was intended in the test_valid_exchange_type - but I think simply replacing warns(None) with one of the suggested alternatives should solve this issue.

@tngTUDOR
Copy link
Contributor

tngTUDOR commented Apr 8, 2024

@BenPortner , can you please confirm that this is related to the main branch (and not the legcy one?)
Also, if you could provide a little reproducible example, it would help to better asses a fix.

This issue talks about the azure pipelines, but overall brightway-lca org is moving to using github only with for CI/CD and one specific task is to move to using the cookiecutter brightwaylib template for all operations. So we would need to fix this issue, not only for the uploading to pypi, but also by using the new CI/CD workflows (i.e.: move away from azure).

use --pre for pip installations to include package dev versions
two tests try to assert that no warnings are raised. the way to do this changed from pytest 7.x to 8.x. new way is copied from https://docs.pytest.org/en/8.0.x/how-to/capture-warnings.html
@BenPortner
Copy link
Member Author

I fixed it. No need to intervene on your side anymore. Answers to your comments below.

@tngTUDOR

@BenPortner , can you please confirm that this is related to the main branch (and not the legcy one?)

You were partly right. Although the pipeline pulled the main branch, I had accidentally deleted the --pre option in the pip commands, causing pip to ignore all dev packages.

Also, if you could provide a little reproducible example, it would help to better asses a fix.

Not sure how to provide a more reproducible example than a pipeline 😅 Pull the code, change something, push and open a PR, see the pipeline fail. Not sure how else to test this...

This issue talks about the azure pipelines, but overall brightway-lca org is moving to using github only with for CI/CD and one specific task is to move to using the cookiecutter brightwaylib template for all operations. So we would need to fix this issue, not only for the uploading to pypi, but also by using the new CI/CD workflows (i.e.: move away from azure).

Not sure why we need this additional layer of complexity. Either way I am strong in favor of fixing the existing pipelines before switching to a more convoluted approach. Otherwise, how are you going to see where things broke?

@michaelweinold

I won't have time to check what was intended in the test_valid_exchange_type - but I think simply replacing warns(None) with one of the suggested alternatives should solve this issue.

This was part of the problem and I fixed it. The other part was caused by pip not installing dev packages because I accidentally deleted the --pre flag.

@BenPortner BenPortner merged commit d2630b0 into main Apr 10, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants