echopype
echopype copied to clipboard
OSOceanAcoustics
Issues with versioning in GitHub builds
After merging #799, portions of our tests are failing. After looking into the failing tests, I found that when we install echopype on our GitHub builds, we get this strange versioning: Successfully installed echopype-0.1.dev1+g8b7249c. In the EchoData class we require a certain form for the versioning, see the below snippet.
https://github.com/OSOceanAcoustics/echopype/blob/8b7249ca06e7a4d91d01ac419d11a9c093e48431/echopype/echodata/echodata.py#L184-L189
For the tests to pass we need to either correct the above code (so that it accounts for this strange versioning) or we need to somehow correct how the GitHub builds are versioning the install of echopype.
@lsetiawan : just bringing this on your radar.
It should be straight forward to use regex instead of split to handle that string parsing. I am curious why the tests didn't have this problem before though -- did something change in the GH actions?
It should be straight forward to use regex instead of split to handle that string parsing. I am curious why the tests didn't have this problem before though -- did something change in the GH actions?
@leewujung after looking through the code, I found that EchoData.version_info is only used in the function ep_version_mapper.py. Additionally, until #799 we had hard coded the version number to 0.6.0, which does not cause an issue in the call EchoData.version_info.
until https://github.com/OSOceanAcoustics/echopype/pull/799 we had hard coded the version number to 0.6.0,
That explains it! But that means we need to fix from the GH actions end.
That explains it! But that means we need to fix from the GH actions end.
Yes, I agree. The best solution would be to make the GH actions install echopype with the version set as the most recent development version.
All the CI tests on PR #799 passed successfully. There are currently no open PR's that are recent, so I checked the most recent closed PR's. #797, closed 3 days ago, also passed all its CI tests.
I do see the incorrect echopype version string all throughout the "Install echopype" section in this CI log in PR #797. But there are no errors reported in those CI test runs.
Did you mean locally run tests? Also, can you specify which tests specifically are failing?
When you install echopype locally, do you get that bad version string too during pip install -e ? This looks to me like a lower-level issue that may go all the way back to setuptools and the related machinery that generate the version string. In fact, I can see that bad string in the CI logs going back as far as July 1 (PR #736). I couldn't go farther back b/c the logs are no longer available.
My assessment is that there's an underlying version string generation problem that's been going on for at least two months but hasn't had any noticeable ramifications anywhere, or at least anything caught by existing tests. PR #799 is simply exposing a problem b/c it introduces a reliance on the version string.
Did you mean locally run tests? Also, can you specify which tests specifically are failing?
@emiliom all PR CI tests should work properly. Locally, everything should be fine too (no bad string with pip install -e). It specifically happens when we merge into dev (I am not sure if it also happens with main or stable). You can see it failing in the GH actions for build.
Got it, thanks.
My observation about the version string being bad as far back as two months ago still applies, though. See this log in a successful build GH action from two months ago: https://github.com/OSOceanAcoustics/echopype/runs/7133791409?check_suite_focus=true#step:12:22 The echopype version string in the log is "0.1.dev1+gaf68509.d20220630"
It's pretty odd that the same context leads to test failures in the build GH actions but not in the PR GH actions.
It's pretty odd that the same context leads to test failures in the build GH actions but not in the PR GH actions.
I agree. On the bright side, it makes me think that there is hopefully a simple solution.
See https://github.com/OSOceanAcoustics/echopype/runs/8218144386?check_suite_focus=true#step:11:94. Now CI installs the correct version. Thanks for catching this.
