Skip to content

Make mmseg to be able to run on mmcv=2.2.0#3663

Open
rabinadk1 wants to merge 3 commits intoopen-mmlab:mainfrom
rabinadk1:patch-1
Open

Make mmseg to be able to run on mmcv=2.2.0#3663
rabinadk1 wants to merge 3 commits intoopen-mmlab:mainfrom
rabinadk1:patch-1

Conversation

@rabinadk1
Copy link
Copy Markdown

@rabinadk1 rabinadk1 commented May 9, 2024

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

The documentation says it is okay to run with mmcv >= 2.2.0, but mmseg incorrectly asserts it to be less than 2.2.0.

Modification

  1. Allow mmcv with version 2.2.0 to run with the latest mmseg package.
  2. Separate the assertion for the package version checks into two statements to provide users with clear feedback on whether their version is less or more than the required range.

BC-breaking (Optional)

No

Use cases (Optional)

N/A

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  3. If the modification has potential influence on downstream projects, this PR should be tested with downstream projects, like MMDet or MMDet3D.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

1. Documentation says it is okay to run with mmcv >= 2.2.0 but mmseg incorrectly asserts.
2. Provide better error messages to pinpoint if the versions are higher or lower
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 9, 2024

CLA assistant check
All committers have signed the CLA.


MMCV_MIN = '2.0.0rc4'
MMCV_MAX = '2.2.0'
MMCV_MAX = '2.3.0'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Directly changing the support version may be dangerous

Copy link
Copy Markdown
Contributor

@MGAMZ MGAMZ left a comment

Choose a reason for hiding this comment

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

LGTM

@karimknaebel
Copy link
Copy Markdown

can we get this merged?

@MGAMZ
Copy link
Copy Markdown
Contributor

MGAMZ commented Jul 23, 2024

can we get this merged?

I believe it's OK, but I am just a contributor without merge permission. This project has been paused for several months.

----From a dissappointed contributor.........

@karimknaebel
Copy link
Copy Markdown

@mmeendez8 @Zoulinx @xiexinch

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.

4 participants