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

Annotation file support #32

Merged

Conversation

gauravparajuli
Copy link
Contributor

hey,

if you want a high level overview on how to use this new feature:
https://github.com/gauravparajuli/pybboxes/tree/annotation-file-support?tab=readme-ov-file#annotation-file-conversion

I included a small coco dataset (196 files) with about 8 megabytes size in the testing directory to check the conversions during unit tests.

currently, also, attempts to either load or write as albumentations and fiftyone raises a not implemented error.

please let me know what you think of this.

Copy link
Owner

@devrimcavusoglu devrimcavusoglu left a comment

Choose a reason for hiding this comment

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

@gauravparajuli Thank you for this amazing work. I've reviewed the code, and have few points on the implementation. We can proceed with the merge after resolving those issues.

all_files.extend(glob.glob(f"{directory}/*{ext}"))
return Counter(file.split('.')[-1] for file in all_files)

sample_yolo_dataset_path = str(os.path.join('tests', 'pybboxes', 'annotations', 'testing_data_yolo'))
Copy link
Owner

Choose a reason for hiding this comment

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

Can we upload these fixtures to a Huggingface ? and the test can run on the fly without the local dependency of the dataset (if exists, use, else, download) (huggingface python sdk already does this caching thing for you, then we will need only path-names or urls here.) I think it would be a nicer way of adding these kind of heavy fixtures.

else:
self._objects[image_name].append(annotation)

def persist_as_yolo(self, export_dir: str):
Copy link
Owner

Choose a reason for hiding this comment

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

can we rename the methods as write_as_... or save_as_... I think it would be better if we stick to the global naming convention with the other python packages. I'd suggest either read/write or load/save, both are fine I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hello, I have made the revisions that you requested for

@devrimcavusoglu
Copy link
Owner

Thanks @gauravparajuli , can you reformat the codebase as instructed at README, to run the tests.

if your local codebase is not added to the PYTHONPATH, you can also run like below at the project root;

python -m tests.run_code_style format

@gauravparajuli
Copy link
Contributor Author

Thanks @gauravparajuli , can you reformat the codebase as instructed at README, to run the tests.

if your local codebase is not added to the PYTHONPATH, you can also run like below at the project root;

python -m tests.run_code_style format

Hi, I have formatted the code

@devrimcavusoglu
Copy link
Owner

Hi @gauravparajuli, can you please recheck you code formatting and apply it again ? It shows error on the code formatting still (see this). You can locally check if the codebase is correctly formatted as follows:

python -m tests.run_code_style check

@gauravparajuli gauravparajuli force-pushed the annotation-file-support branch from f0c9020 to 904558c Compare October 6, 2024 09:52
Copy link
Owner

@devrimcavusoglu devrimcavusoglu left a comment

Choose a reason for hiding this comment

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

Thanks for this amazing contribution 🎊 @gauravparajuli

@devrimcavusoglu devrimcavusoglu merged commit bcd2b18 into devrimcavusoglu:main Oct 6, 2024
12 checks passed
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.

2 participants