Skip to content

Conversation

@Opdoop
Copy link
Contributor

@Opdoop Opdoop commented Nov 20, 2020

Thanks for releasing the powerful TextAttack🎉
For chinese fellows easier to get started with TextAttack, I translate the reademe to chinese. I try my best to adheres to 信、达、雅, the translation principles. But only the content is guaranteed to be informative. This chinese version may need some major review. Despite all this, it's still a good startpoint.
Look forward to receiving your reply~

@qiyanjun
Copy link
Member

@MintForever please help review this!

@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 22, 2020

@qiyanjun Thanks for the reply~
And @MintForever, waiting for your constructive comment~

@qiyanjun
Copy link
Member

@Opdoop can you help me also remove those HTML inline styles from the recipe table in the main Readme.md?

BTW: please rebase! I did some other changes after your PR! https://textattack.readthedocs.io/en/latest/1start/support.html#start-contributing-pull-requests

@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 22, 2020

Sorry, I'm a newbie at git. I'm not sure how to rebase after a PR. 😅
@qiyanjun I'll try it tomorrow.🧐

Opdoop and others added 26 commits November 22, 2020 16:33
As https://github.com/github/markup first step (2) describe Github html will be sanitized and all inline-style will be removed. So this inline style in readme is useless.

Signed-off-by: Opdoop <[email protected]>
As https://github.com/github/markup first step (2) describe Github html will be sanitized and all inline-style will be removed. So this inline style in readme is useless.

Signed-off-by: Opdoop <[email protected]>
@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 23, 2020

@qiyanjun Aloha~ I think I did it.
And remove all in-line style and class="{odd}{even}" didn't change the appearance of the tables.

@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 23, 2020

@MintForever Hi, I noticed the review_readme_zh branch, and thanks for your revision! ❤
Only a minor suggestion, the dash '——' in the HTML table title is used to fix the width of the column. Because the github preferred markdown wrap html table column by the max number of word of that column. Without this —— some columns in the table were wrapped in an ugly way. Wish you figure out an elegant way. Or it's better to keep this dash.

Signed-off-by: Opdoop <[email protected]>
@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 23, 2020

🧐Question: Where should the entry of chinese version readme be placed?

@qiyanjun
Copy link
Member

@Opdoop have you solved @MintForever 's concern?

@Opdoop I will add a link in the main READMe.md

@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 23, 2020

@qiyanjun Hi, I did get it. @MintForever his review commits are on branch review_readme_zh, do you want me to merge his change in this PR?

@qiyanjun
Copy link
Member

@Opdoop please merge @MintForever (her) change into the PR

@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 24, 2020

@qiyanjun Aha~ Done~ Merged @MintForever (her) change into the PR 😃

@qiyanjun
Copy link
Member

@Opdoop thanks. Last request of change: I have revised the ordering under the "Design" section. Can you revise your Chinese version accordingly? Many thanks,

@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 25, 2020

@qiyanjun Changed the order under the design section and added the Benchmarking section.
I see you're trying to separate dataset and model into two sections. That's great! But HuggingFace support and Loading a model or dataset from a file are still in a coupling.

@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 25, 2020

I only modified README_ZH.md. I have no idea why the Test with PyTest failed.🤦‍♂️

@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 25, 2020

The PyTest failing checks at install dependencies and the log is below:

pip._vendor.urllib3.exceptions.ProtocolError: ("Connection broken: ConnectionResetError(104, 'Connection reset by peer')", ConnectionResetError(104, 'Connection reset by peer'))
Error: Process completed with exit code 2.

Still have no idea why this happened.

@qiyanjun
Copy link
Member

@Opdoop Thanks.

@qiyanjun qiyanjun merged commit 6a383bf into QData:master Nov 25, 2020
@Opdoop
Copy link
Contributor Author

Opdoop commented Nov 25, 2020

@qiyanjun Thank you for your valuable comments and suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants