SEACrowd / seacrowd-datahub

A collaborative project to collect datasets in SEA languages, SEA regions, or SEA cultures.
Apache License 2.0
55 stars 54 forks source link

Closes #569 | Add/Update Dataloader MSL4Emergency #677

Closed sabilmakbar closed 1 month ago

sabilmakbar commented 1 month ago

Please name your PR title and the first line of PR message after the issue it will close. You can use the following examples:

Closes #569

Checkbox

sabilmakbar commented 1 month ago

Somehow, I still get error when running .load_dataset or testing via tests.test_seacrowd, the error is:

UnicodeDecodeError: 'charmap' codec can't decode byte 0x81 in position 11: character maps to <undefined>.

Do you know why this happens? Or is everything fine (OK/passed) on ​​your side? @sabilmakbar

Hmm, on my end it works fine. You can check this log file, which is a sh output from this cmd: python -m tests.test_seacrowd seacrowd/sea_datasets/msl4emergency/msl4emergency.py

test_msl4emergency.log

I'm thinking that your error might come from these lines (in L114-115):

with open(text_path, "r") as f:
    text_data = [data.split("\t") for data in f.readlines()]

You can try to print the text_path var and load it separately on your end to see if that's indeed the case.

Update: Oh perhaps it's related to _CITATION strings (if the line indicator is referring to the string decoding error in Citation lines).

let me know any of these solved the bug on this code @muhammadravi251001

muhammadravi251001 commented 1 month ago

It's not about the _CITATION. After changing the encoding parameter on open to:

with open(text_path, "r", encoding="utf-8") as f:

I get an OK response and get the dataset perfectly with load_dataset() method.

The problem is: that I run this dataloader implementation on my Windows notebook, and Windows does not always run encoding on utf-8 in my case; while Linux and MacOS default is utf-8.

To avoid this future-encoding-error, I recommend adding an encoding value to the open method, @sabilmakbar .

sabilmakbar commented 1 month ago

thanks! done making changes, pls have a look @muhammadravi251001

holylovenia commented 1 month ago

Hi @ljvmiranda921, sorry for the sudden notice. I assigned you here in place of @luckysusanto due to the lack of response.

I would like to let you know that we plan to finalize the calculation of the open contributions (e.g., dataloader implementations) in 31 hours, so it'd be great if we could wrap up the reviewing and merge this PR before then.

ljvmiranda921 commented 1 month ago

Sure! I'll just update the small nits myself as well just to speed things up! 👍