search

roboflow / roboflow-python

The official Roboflow Python package. Manage your datasets, models, and deployments. Roboflow has everything you need to build a computer vision application.
https://docs.roboflow.com/python
Apache License 2.0
271 stars 70 forks source link

Bugfix with .json not returning valid json and added .dict #252

Closed Qfc9 closed 1 month ago

Qfc9 commented 4 months ago

Description

We are currently facing an issue with the .predict function which includes an option for .json. However, this .json method does not generate valid JSON data; rather, it returns a dictionary. This discrepancy is likely to cause confusion and issues for users.

The necessary adjustments are to be implemented in the PredictionGroup class within the utils module.

No additional dependencies are required for this change.

Type of Change

Testing and Validation

This change has been executed and verified through direct code implementation. The modification entails relocating the currently effective code to a .dict() method and subsequently converting its output into valid JSON within the .json method.

Deployment Considerations

Potential issues might arise for users who are currently utilizing the output from the .json method (which is effectively a dictionary) as a dictionary in their code. These users may need to adjust their implementations to accommodate the changes.

CLAassistant commented 3 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: Qfc9
:x: pre-commit-ci[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

capjamesg commented 2 months ago

@Qfc9 Hello there! Do you have a code example showing a test case for this project?

iurisilvio commented 1 month ago

I agree it might be misleading, but I think it is a common pattern in Python to call it .json() instead of .dict(). It is something like "data prepared to be JSON".

This change would be very backward incompatible with little value, I prefer to not do it, maybe adding type hints and better docs to the method would fix the confusion.