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

Fix RecursionError in Item class #732

Merged
merged 2 commits into from Nov 28, 2023
Merged

Fix RecursionError in Item class #732

merged 2 commits into from Nov 28, 2023

Conversation

saurbhc
Copy link
Member

@saurbhc saurbhc commented Nov 23, 2023

Problem

Printing the Item class raises RecursionError

from darwin.future.meta.client import Client

dataset = client.team.datasets.collect()[-1]
print(dataset.items.where().collect())
Traceback (most recent call last):
  File "t4.py", line 15, in <module>
    raise SystemExit(main())
  File "t4.py", line 8, in main
    print(dataset.items.where().collect())
  File "/Users/saurabhchopra/github/v7labs/darwin-py/darwin/future/meta/objects/base.py", line 50, in __repr__
    return str(self)
  File "/Users/saurabhchopra/github/v7labs/darwin-py/darwin/future/meta/objects/base.py", line 50, in __repr__
    return str(self)
  File "/Users/saurabhchopra/github/v7labs/darwin-py/darwin/future/meta/objects/base.py", line 50, in __repr__
    return str(self)
  [Previous line repeated 329 more times]
RecursionError: maximum recursion depth exceeded while getting the str of an object

Solution

Implement the missing __str__ method in Item class, which solves the issue.

Changelog

Fix RecursionError while printing an Item class.

@saurbhc saurbhc added the bug Something isn't working label Nov 23, 2023
@saurbhc saurbhc self-assigned this Nov 23, 2023
@saurbhc saurbhc marked this pull request as ready for review November 23, 2023 09:24
Copy link
Contributor

@Nathanjp91 Nathanjp91 left a comment

Choose a reason for hiding this comment

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

Interested to understand why that causes a recursion error, and if it's general to all inherited meta objects, seeking to fix that (or enforce str as an abstractmethod), but good to approve this small fix.

@saurbhc
Copy link
Member Author

saurbhc commented Nov 23, 2023

Interested to understand why that causes a recursion error

From what I could see, it may be coming from the Base class -> __repr__ method [link] which was calling the str implementation - as there was no __str__ defined in Item class, it re-tried in a loop calling itself in recursion?

@Nathanjp91
Copy link
Contributor

Interested to understand why that causes a recursion error

From what I could see, it may be coming from the Base class -> __repr__ method [link] which was calling the str implementation - as there was no __str__ defined in Item class, it re-tried in a loop calling itself in recursion?

Can you add in a default implementation to __str__ of that Base class for this PR, just print the underlying element it manages

@Nathanjp91 Nathanjp91 merged commit 09b00d2 into master Nov 28, 2023
13 checks passed
@saurbhc saurbhc deleted the item-str-fix branch November 28, 2023 10:39
Nathanjp91 added a commit that referenced this pull request Dec 12, 2023
* fixing test

* added polygon and complex polygon tests with bounding boxes

* extended convertion tests

* formatter

* updated test to reflect adding of bounding box

* updated tests to check for new format

* added additional test for box and tag

* removed prints

* black reformat

* black format

* removed an import

* added schema ref

* reformated utils

* added support RemoteDatasetV1 parsing and updated tests

* black fomrat

* additional black magic

* fixed conflic

* minor fixes

* removed ignore of empty files

* updated tests for new (old) behaviour

* added test case

* updated test

* updated code to work with bounding boxes and polygon, also added toggle to use all or only non empty annotations to local dataset class

* black

* adjusting paths to pass tests

* black

* handling polyg and bbox bounding box anno

* [PY-401][external] Restore Meta Item & ItemQuery functions (#718)

* Meta functions & tests to restore items

* Test fixes

* [PY-402][external] Archive Meta Item & ItemQuery functions (#719)

* Meta functions & tests to archive items

* Test improvements

* Linting

* [PY-403][external] Set Priority Meta Item & ItemQuery functions (#720)

* Fixed missing path parameter in core move items to folder method

* Fixed missing priority parameter in core set item priority method

* Linting

* Meta Item & ItemQuery functions to move between folders

* Update darwin/future/tests/meta/queries/test_item.py

Co-authored-by: Owen Jones <owencjones+github@gmail.com>

* Added sad path tests

* Meta functions & tests to set item priority

* Linting

---------

Co-authored-by: Owen Jones <owencjones+github@gmail.com>

* automatic ruff --fix changes (#723)

* automatic ruff --fix changes

* black changes

* revert of client

* minor updates to tests

* added make polygon tests for darwin_v2 format

* updating tests to accomidate darwin V2 format

* black

* added nifty V2 test

* black

* refactor

* ruff --fix

* removed try catch in stacked targets

* removed print

* Reverted specific files to state in commit 09b105e

* converting complex and regular polygon to import format

* added a potential e2e import fix

* removed debug prints

* latest sync

* updated code to pass e2e tests

* black and ruff --fix all

* updated formatting

* minor changes to complex polygon

* minor fix

* black

* minor updates based on comments

* added PolygonPath and PolygonPaths definitions

* merge

* extended convertion tests

* local changes

* reverted changes to internal darwin format, convertion to v2 only done when writing to file now

* black

* ruff and black

* removed settings.json changes

* reverting to non-v2 data.zip

* merged darwin_v1_test changes from origin master

* changed json stream to normal json

* Fix `RecursionError` in `Item` class (#732)

* Item missing `__str__` method raises `RecursionError`

* update `MetaBase` class - add the missing `__str__` method

* commiting changes

* fixed json-stream error when checking for non-empty lists

* remove unused import

* fixed video to image convertion bug when folders are used

* removed debug print

* removed debug print

---------

Co-authored-by: John Wilkie <124276291+JBWilkie@users.noreply.github.com>
Co-authored-by: Owen Jones <owencjones+github@gmail.com>
Co-authored-by: Nathan Perkins <nathanjp91@gmail.com>
Co-authored-by: saurbhc <sc@saurabhchopra.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants