brightway-lca / brightway2-data

Tools for the management of inventory databases and impact assessment methods. Part of the Brightway LCA framework.
https://docs.brightway.dev/
BSD 3-Clause "New" or "Revised" License
11 stars 24 forks source link

Fix pipeline #164

Closed BenPortner closed 7 months ago

BenPortner commented 8 months ago

@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 commented 7 months ago

@tngTUDOR @michaelweinold anything you can do?

michaelweinold commented 7 months ago

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

BenPortner commented 7 months ago

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

michaelweinold commented 7 months ago

@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 commented 7 months ago

@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).

BenPortner commented 7 months ago

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.