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

Remove coco and ssd #1717

Merged
merged 38 commits into from
Dec 12, 2022
Merged

Remove coco and ssd #1717

merged 38 commits into from
Dec 12, 2022

Conversation

growlix
Copy link
Contributor

@growlix growlix commented Nov 8, 2022

What does this PR do?

This PR removes COCO and SSD due to lack of use and maintenance. Deyahpification seemed like a good opportunity to pull the plug. From the relevant ticket:

I’m not sure if it would be worth the time to deyahpify COCO since it may not be easy to test. The only model that can use it is SSD, but no one has used this for sometime so it may not work. In the future, I think we want to move to pulling models from mmdetection which will likely require the COCO to be refactored to output a different data structure. Since we anticipate a refactor in the future, it may be worth delaying the deyahp until then.

Before submitting

  • Have you read the contributor guidelines?
  • Is this change a documentation change or typo fix? If so, skip the rest of this checklist.
  • Was this change discussed/approved in a GitHub issue first? It is much more likely to be merged if so.
  • Did you update any related docs and document your change?
  • Did you update any related tests and add any new tests related to your change? (see testing)
  • Did you run the tests locally to make sure they pass?
  • Did you run pre-commit on your change? (see the pre-commit section of prerequisites)

Copy link
Contributor

@A-Jacobson A-Jacobson left a comment

Choose a reason for hiding this comment

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

I'm cool with it. 100% agree with what you quoted. Did i say that?

Copy link
Contributor

@karan6181 karan6181 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Copy link
Contributor

@Landanjs Landanjs left a comment

Choose a reason for hiding this comment

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

LGTM! Another 1.3k LOC down

@growlix growlix enabled auto-merge (squash) December 12, 2022 20:01
@growlix growlix merged commit ba8fa94 into mosaicml:dev Dec 12, 2022
@growlix growlix deleted the no-coco branch December 12, 2022 22:21
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.

5 participants