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

feat : added support for keypoints dataset #1477

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

shaddu
Copy link
Contributor

@shaddu shaddu commented Aug 24, 2024

Description

Added support for keypoints dataset

List any dependencies that are required for this change.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How has this change been tested, please provide a testcase or example of how you tested the change?

Keypotdataset Colab

Any specific deployment considerations

For example, documentation changes, usability, usage/costs, secrets, etc.

Docs

  • Docs updated? What were the changes:

@onuralpszr
Copy link
Collaborator

First suggestion I would say we should split the code to 2 files detections and keypoints and maybe also add base too because file getting really bigger and mix both detections and keypoints not the way I would do it. I would like to hear from others as well.

cc @LinasKo @SkalskiP

@LinasKo
Copy link
Contributor

LinasKo commented Aug 26, 2024

@onuralpszr, you're very right that it needs a split. It's a massive file.

Let's do that right after the next release, but merge this PR as is.
(after I review it)

@shaddu, fantastic work, as always.

I have one request early request: please add types to each of the split methods, for both classification, detection and keypoint dataset.

    def split(
        self,
        split_ratio: float = 0.8,
        random_state: Optional[int] = None,
        shuffle: bool = True,

Another PR will introduce this change soon, so adding it here will pre-empt the conflict.

@onuralpszr onuralpszr added the api:datasets Dataset API label Aug 27, 2024
@shaddu
Copy link
Contributor Author

shaddu commented Aug 27, 2024

Hi @onuralpszr and @LinasKo ,

I also feel that files should be split based on keypoints and detection types as there could be confusion while working with annotations.

@LinasKo I have added type to the split method for keypoint dataset and can see you have already added for the rest.

Thank you for reviewing the PR ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api:datasets Dataset API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants