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

try to fix CI problem #3142

Merged
merged 1 commit into from Sep 16, 2022
Merged

try to fix CI problem #3142

merged 1 commit into from Sep 16, 2022

Conversation

megies
Copy link
Member

@megies megies commented Sep 16, 2022

What does this PR do?

Changes obspy installation in CI from editable to a regular pip installation.

Also remove the [tests] modifier, which does nothing in combination with --no-deps anyway and we rely on an environment file for all (testing) dependencies anyway.

Why was it initiated? Any relevant Issues?

Some test currently fails because files are being looked up in the install location, which is only holding a link to the source tree though, due to being a pip editable installation.

PR Checklist

  • Correct base branch selected? master for new features, maintenance_... for bug fixes
  • This PR is not directly related to an existing issue (which has no PR yet).
  • If the PR is making changes to documentation, docs pages can be built automatically.
    Just add the "build_docs" tag to this PR.
    Docs will be served at docs.obspy.org/pr/{branch_name} (do not use master branch).
    Please post a link to the relevant piece of documentation.
  • If all tests including network modules (e.g. clients.fdsn) should be tested for the PR,
    just add the "test_network" tag to this PR.
  • All tests still pass.
  • Any new features or fixed regressions are covered via new tests.
  • Any new or changed features are fully documented.
  • Significant changes have been added to CHANGELOG.txt .
  • First time contributors have added your name to CONTRIBUTORS.txt .
  • If the changes affect any plotting functions you have checked that the plots
    from all the CI builds look correct. Add the "upload_plots" tag so that plotting
    outputs are attached as artifacts.
  • Add the "ready for review" tag when you are ready for the PR to be reviewed.

seems one test is looking for test data in the wrong place. obspy is
installed with pip as editable, so in the Python tree there is only a
link and setuptools plugin system still points us to that path that only
has the link for plugin folders?
@megies megies added the CI issue generally related to continuous integration label Sep 16, 2022
@megies
Copy link
Member Author

megies commented Sep 16, 2022

@trichter not sure if there was a reason for doing an editable install in CI, but it started to cause issues and it looks like a regular install does just fine..

@megies megies merged commit 166e92c into maintenance_1.3.x Sep 16, 2022
@megies megies deleted the fix_gh_actions branch September 16, 2022 11:56
@trichter
Copy link
Member

I think for no specific reason.

@trichter
Copy link
Member

What I do not understand is, why this error appeared all of a sudden in the CI setup. I myself use this setup for my obspy installation. Will report when/if it breaks.

@megies
Copy link
Member Author

megies commented Sep 26, 2022

I use editable installs all the time too, locally. But CI can have some special setups makin it weird, looks like auto stripping of test file directries maybe? Who knows..

@megies
Copy link
Member Author

megies commented Oct 12, 2022

I actually can reproduce the problem locally now.. happens when using a pip editable install and then going via obspy-runtests.

@trichter
Copy link
Member

For my local installation it is still working. Maybe a change in the entry points stuff of setuptools. Should we revert this PR?

@megies
Copy link
Member Author

megies commented Oct 13, 2022

For my local installation it is still working. Maybe a change in the entry points stuff of setuptools. Should we revert this PR?

The underlying problem was fixed by #3175. On a fresh installation with all the newest packages and installing obspy editable after reverting that commit, it fails. As you say, seems to be a change in setuptools, reporting the shimmy directory instead of the git repository location as install base dir.

Anyway, I dont care, if you want to revert this go ahead.

EDIT: Ah wait, I thought you meant reverting the CI script to be an editable install again. We can not revert this PR, like stated, obspy-runtests on an editable install still fails if you revert this

@megies
Copy link
Member Author

megies commented Oct 13, 2022

to reproduce:

conda create -n trash obspy
conda activate trash
conda remove --force --offline obspy
pip install -e /path/to/obspy
# manually revert this commit
obspy-runtests

@megies
Copy link
Member Author

megies commented Oct 13, 2022

Oh boy.. too many swapping around tabs and issues.. @trichter yes this change in CI script can be changed back to editable install if you want

@trichter
Copy link
Member

It's OK. I don't care enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI issue generally related to continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants