close
The Wayback Machine - https://web.archive.org/web/20211224110220/https://github.com/jina-ai/jina/issues/3661
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

from_csv not robust for real-world datasets #3661

Open
alexcg1 opened this issue Oct 13, 2021 · 5 comments
Open

from_csv not robust for real-world datasets #3661

alexcg1 opened this issue Oct 13, 2021 · 5 comments

Comments

@alexcg1
Copy link
Member

@alexcg1 alexcg1 commented Oct 13, 2021

Describe the bug
I'm having major trouble with from_csv.

Context: I'm writing tutorial for build simple text search engine with Jina + Hub. I don't want to include a whole section of processing datasets, hence just passing a CSV into from_csv. I tried with meme dataset (converted tsv) before, and now using superhero dataset (native csv). Still getting same issues.

What I want to do:
Document.text to be populated from the field powers_text
Document.tags to be every other field in the record

My code:

with flow, open("data/superheroes.csv") as fp:
    flow.index(from_csv(fp, field_resolver={"powers_text": "text"}), size=10)

Expected result:

  • Clean load of each row of CSV as individual Document
  • Any escaping/quoting handled automatically

Actual result:

ina.excepts.BadDocType: fail to construct a document from {'name': '3-D Man', 'real_name': 'Delroy Garrett, Jr.', 'full_name': 'Delroy Garrett, Jr.', 'overall_score': '6', 'history_text': "D
elroy Garrett, Jr. grew up to become a track star and competed in the Olympic Games. After he tested positive for steroids, he lost his three gold medals. In despair, he turned to religion, s
pecifically the Triune Understanding. This group's founder, Jonathan Tremont, had found one of three fragments of a mysterious object and used it to give Garrett the powers of 3-D Man. Garret
t assumed that his powers were because of his newfound spiritual enlightenment, and Tremont never disabused him of the notion.", 'text': '', 'intelligence_score': '85', 'strength_score': '30'
, 'speed_score': '60', 'durability_score': '60', 'power_score': '40', 'combat_score': '70', 'superpowers': "['Super Speed', 'Super Strength']", 'alter_egos': '[]', 'aliases': "['']", 'place_o
f_birth': '', 'first_appearance': '', 'creator': 'Marvel Comics', 'alignment': 'Good', 'occupation': '', 'base': '', 'teams': "['Annihilators', 'Asgardians', 'Avengers', 'New Avengers']", 're
latives': '', 'gender': 'Male', 'type_race': 'Human', 'height': '-', 'weight': '-', 'eye_color': '', 'hair_color': '', 'skin_color': '', 'img': '/pictures2/portraits/11/050/10038.jpg?v=156096
9486', 'has_electrokinesis': '0.0', 'has_energy_constructs': '0.0', 'has_mind_control_resistance': '0.0', 'has_matter_manipulation': '0.0', 'has_telepathy_resistance': '0.0', 'has_mind_contro
l': '0.0', 'has_enhanced_hearing': '0.0', 'has_dimensional_travel': '0.0', 'has_element_control': '0.0', 'has_size_changing': '0.0', 'has_fire_resistance': '0.0', 'has_fire_control': '0.0', '
has_dexterity': '0.0', 'has_reality_warping': '0.0', 'has_illusions': '0.0', 'has_energy_beams': '0.0', 'has_peak_human_condition': '0.0', 'has_shapeshifting': '0.0', 'has_heat_resistance': '
0.0', 'has_jump': '0.0', 'has_self-sustenance': '0.0', 'has_energy_absorption': '0.0', 'has_cold_resistance': '0.0', 'has_magic': '0.0', 'has_telekinesis': '0.0', 'has_toxin_and_disease_resis
tance': '0.0', 'has_telepathy': '0.0', 'has_regeneration': '0.0', 'has_immortality': '0.0', 'has_teleportation': '0.0', 'has_force_fields': '0.0', 'has_energy_manipulation': '0.0', 'has_endur
ance': '0.0', 'has_longevity': '0.0', 'has_weapon-based_powers': '0.0', 'has_energy_blasts': '0.0', 'has_enhanced_senses': '0.0', 'has_invulnerability': '0.0', 'has_stealth': '0.0', 'has_mark
smanship': '0.0', 'has_flight': '0.0', 'has_accelerated_healing': '0.0', 'has_weapons_master': '0.0', 'has_intelligence': '0.0', 'has_reflexes': '0.0', 'has_super_speed': '1.0', 'has_durabili
ty': '0.0', 'has_stamina': '0.0', 'has_agility': '0.0', 'has_super_strength': '1.0'}, if you are trying to set the content you may use "Document(content=your_content)"

Describe how you solve it

Write a CSV loader by hand, using Python's csv library. Which works right away out of the box, so I think from_csv should be able to work better.

docs = DocumentArray()

with open("data/superheroes.csv", "r") as data:
    reader = csv.reader(data, delimiter=",")
    for line in csv.DictReader(data):
        doc = Document(text=line["powers_text"])
        doc.tags = line
        docs.append(doc)

Nice to haves

While we're at it, it'd be really nice to have a delimiter parameter so we could use `from_csv('foo.csv', delimiter='\t') to load TSVs etc


Environment

Screenshots

@ybv
Copy link

@ybv ybv commented Oct 16, 2021

@alexcg1 @hanxiao @JoanFM 👋🏽 I'm new to Jina, and I'd love to help fix this

Based on my investigation, it looks like the dataset has a field, weight which intersects with one of the meta fields on the Document , resulting in the following js_dict

{'text': '', 'weight': '-'}

I think this could be resolved by either doing validation on meta fields before parsing into Protobufs or by ignoring other fields when a field_resolver is specified, let me know if you can think of other alternative solutions.

@bwanglzu
Copy link
Member

@bwanglzu bwanglzu commented Oct 18, 2021

@ybv thanks for your investigate, if you can help that would be great!

  1. I think a validation between csv fields and meta fields sounds good to me.
  2. if possible, can you also add the delimiter parameter as was suggested by @alexcg1 ?

@askalik
Copy link

@askalik askalik commented Oct 24, 2021

For the delimiter parameter wouldn’t you just add in a new optional parameter on the from_csv function that has a default to “,” in https://github.com/jina-ai/jina/blob/86a6b987f3d5f3c80a612e37ece4b8411b535ac2/jina/types/document/generators.py

@alexcg1
Copy link
Member Author

@alexcg1 alexcg1 commented Oct 25, 2021

For the delimiter parameter wouldn’t you just add in a new optional parameter on the from_csv function that has a default to “,” in 86a6b98/jina/types/document/generators.py

Yup, this seems like a logical way to do it

@ggdupont
Copy link
Contributor

@ggdupont ggdupont commented Oct 28, 2021

Reproducing the error locally, I have a different source error:

google.protobuf.json_format.ParseError: Failed to parse weight field: Couldn't parse float: -..

For the 3-D Man row of the file, the weightis set to - (no quote) which seems to be interpreted as a float.

I mean the intersection of fields name is valid, just the problem seems to be on the value side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants