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

SVT dataset integration #597

Closed
wants to merge 14 commits into from
Closed

Conversation

felixdittrich92
Copy link
Contributor

@felixdittrich92 felixdittrich92 commented Nov 9, 2021

This PR integrates the Street View Text dataset

@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #597 (28e1a47) into main (400aec0) will decrease coverage by 0.11%.
The diff coverage is 97.05%.

❗ Current head 28e1a47 differs from pull request most recent head 7c3505e. Consider uploading reports for the commit 7c3505e to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #597      +/-   ##
==========================================
- Coverage   96.16%   96.04%   -0.12%     
==========================================
  Files         111      111              
  Lines        4300     4299       -1     
==========================================
- Hits         4135     4129       -6     
- Misses        165      170       +5     
Flag Coverage Δ
unittests 96.04% <97.05%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
doctr/datasets/svt.py 96.96% <96.96%> (ø)
doctr/datasets/__init__.py 100.00% <100.00%> (ø)
doctr/utils/data.py 88.67% <0.00%> (-9.44%) ⬇️
doctr/datasets/doc_artefacts.py 93.33% <0.00%> (-0.79%) ⬇️
doctr/datasets/iiit5k.py
doctr/datasets/datasets/base.py 95.55% <0.00%> (+0.10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 400aec0...7c3505e. Read the comment docs.

@felixdittrich92 felixdittrich92 marked this pull request as ready for review November 10, 2021 07:45
@felixdittrich92 felixdittrich92 changed the title [WIP] SVT dataset integration SVT dataset integration Nov 10, 2021
Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

Thanks! I added some optimization comment & 1 about XML security

doctr/datasets/detection.py Outdated Show resolved Hide resolved
doctr/datasets/detection.py Outdated Show resolved Hide resolved
doctr/datasets/recognition.py Outdated Show resolved Hide resolved
doctr/datasets/recognition.py Outdated Show resolved Hide resolved
doctr/datasets/svt.py Show resolved Hide resolved
doctr/datasets/svt.py Outdated Show resolved Hide resolved
doctr/datasets/svt.py Outdated Show resolved Hide resolved
doctr/datasets/svt.py Outdated Show resolved Hide resolved
doctr/datasets/svt.py Outdated Show resolved Hide resolved
@fg-mindee fg-mindee self-assigned this Nov 10, 2021
@fg-mindee fg-mindee added the module: datasets Related to doctr.datasets label Nov 10, 2021
Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

Still got many questions 🙃

doctr/datasets/svt.py Outdated Show resolved Hide resolved
(int(rect_tag.attrib['x']) + int(rect_tag.attrib['width']) / 2,
int(rect_tag.attrib['y']) + int(rect_tag.attrib['height']) / 2,
int(rect_tag.attrib['width']), int(rect_tag.attrib['height']), 0)
for rect_tag in image_tag
Copy link
Contributor

Choose a reason for hiding this comment

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

according to what you said above, I don't see why we don't need to check that if image_tag.tag == 'taggedRectangles' then 🤔

Copy link
Contributor Author

@felixdittrich92 felixdittrich92 Nov 10, 2021

Choose a reason for hiding this comment

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

@fg-mindee

image.tag : imageName
image.attrib : {}
image.text : img/04_16.jpg
image.tag : address
image.attrib : {}
image.text : 324 South Glenoaks Boulevard Burbank CA 91502-1318
image.tag : lex
image.attrib : {}
image.text : BLOCKBUSTER,VIDEO,LAW,OFFICES,RAY,FOUNTAIN,CRIMINAL,DEFENSE,ATTORNEY,LAWYER,PIZZA,MAN,CAFE,COLOMBIA,GERRO,MARSHA,RICO,GLENOAKS,PHARMACY,AND,MEDICAL,SUPPLY,RAMIREZ,LYNN,TER,POGHOSYAN,ZARINE,JAS,GIFTS,KNOX,TIMOTHY,DDS,LILLY,PARK,HOME,THEATER,HAMOUI,MOBIL,MART,CLEANERS,TOKYO,YAKIDORI,SHIRVAN,REALTY,GROUP,NAPA,AUTO,CARE,CENTER,TRAN,DEBBY
image.tag : Resolution
image.attrib : {'x': '1280', 'y': '1024'}
image.text : None
image.tag : taggedRectangles
image.attrib : {}
image.text : 

This is for one example hopefully it makes it a bit clearer

Why we need no check ?
taggedRectangles has only a attribut
and tag with the label is child from taggedRectangles
but in image_tag we have multible tags: imageName, address, lex, Resolution and taggedRectangles

<taggedRectangle height="31" width="120" x="351" y="455">
<tag>MAGIC</tag>
</taggedRectangle>

with xpath we can search in the tree:
element = (x.findall('.//td[@class="teamid"]'))
but in this case i would prefer the if statement ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, but I'm not really familiar with XML, so I have no clue in your snippet whether there is some hierarchy, whether this is a flat data structure or whether we can access attribute values by name 😅

Generally speaking, nested FOR loops have to be avoided, hence my previous question ;)
So, in Python, when processing the XML, for what I understand:

  • xml_root holds the list of pages/images
  • in each page/image, there are several attributes, and we're interested in "imageName", "x", "y", "width", "height" and those are on the same level

Is this correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

j
No lets try to explain with this snippet:
tagset is root
image is first depth tag
imageName, address, lex, Resolution and taggedRectangles is second depth (x,y from Resolution are attributes and the other are text)
taggedRectangles has 1 more depth taggedRectangle with attributes height, width, x, y
and each taggedRectangle has one more depth tag with a text value
@fg-mindee
i have also tryed pythons built-in import xmltodict but this does not work in fact that the xml file is not well formated :/
the other way (and i think the only other way) with XPath something like this: .//imageName but for readability i would prefer the actual implementation 😅
wdyt ?

@fg-mindee fg-mindee added topic: documentation Improvements or additions to documentation ext: tests Related to tests folder labels Nov 10, 2021
felixdittrich92 added a commit to felixdittrich92/doctr that referenced this pull request Nov 13, 2021
felixdittrich92 added a commit to felixdittrich92/doctr that referenced this pull request Nov 15, 2021
fg-mindee pushed a commit that referenced this pull request Nov 15, 2021
* start synth

* cleanup

* reopening #597

* apply changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext: tests Related to tests folder module: datasets Related to doctr.datasets topic: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants