Fix incorrect `eq` method of `AttackedText` in `textattack/shared/attacked_text.py` #509

wenh06 · 2021-08-03T15:04:19Z

What does this PR do?

Summary

fix incorrect __eq__ method of AttackedText in textattack/shared/attacked_text.py

~~Another thing is that I added DownloadConfig in textattack/datasets/huggingface_dataset.py to try to use a proxy for downloading things from huggingface. This is irrelevant to this pull request.~~

Additions

in the __eq__ method of AttackedText, an additional check of equal number of dict items of attack_attrs is added. If this correction was not done, the __eq__ is incorrect, as illustrated in the following example

from textattack.shared.attacked_text import AttackedText
at1 = AttackedText("hehe", attack_attrs={"hehe":1})
at2 = AttackedText("hehe", attack_attrs={"hehe":1, "haha":2})

print(at1 == at2)
print(at2 == at1)

whose outcome would be

True
False

Changes

NA

Deletions

NA

Checklist

[√] The title of your pull request should be a summary of its contribution.
[√] Please write detailed description of what parts have been newly added and what parts have been modified. Please also explain why certain changes were made.
[√] If your pull request addresses an issue, please mention the issue number in the pull request description to make sure they are linked (and people consulting the issue know you are working on it)
[√] To indicate a work in progress please mark it as a draft on Github.
[√] Make sure existing tests pass.
[√] Add relevant tests. No quality testing = no merge.
[√] All public methods must have informative docstrings that work nicely with sphinx. For new modules/files, please add/modify the appropriate .rst file in TextAttack/docs/apidoc.'

…cked_text.py

minor format issue

qiyanjun · 2021-08-06T14:20:26Z

@cogeid please review

qiyanjun · 2021-08-21T18:26:37Z

@wenh06 thanks for the PR.

What is the purpose of "added DownloadConfig in textattack/datasets/huggingface_dataset.py to try to use a proxy for downloading things from huggingface."?

What purpose is this addition for?

wenh06 · 2021-08-22T02:27:31Z

@wenh06 thanks for the PR.

What is the purpose of "added DownloadConfig in textattack/datasets/huggingface_dataset.py to try to use a proxy for downloading things from huggingface."?

What purpose is this addition for?

My direct connection to github (huggingface datasets download things from raw.githubusercontent) is usually poor, so I added this. If it is useless to most users, I think it can be removed. Thank you.

sanchit97 · 2021-09-02T20:58:46Z

@wenh06 Thank you for the PR!

The __eq__ method's check was an oversight, we should check both directions.
The index bug in first_word_diff was also an oversight, mostly because the method is never referenced, its an added utility.
DownloadConfig can be removed in the huggingface_dataset.py file. Can you remove them from the PR?

@qiyanjun I think the changes in attacked_text.py are ready to merge.

download config was added in previous 2 commits, which is useless for most of the people, hence is removed.

wenh06 · 2021-09-03T00:12:19Z

3. DownloadConfig is removed now.

sanchit97 · 2021-09-03T00:20:58Z

@wenh06 Thanks a lot! Ready to merge after checks.

qiyanjun · 2021-09-03T18:28:25Z

@wenh06 can you fix the formatting errors / then the pytest errors? thanks

wenh06 · 2021-09-04T15:00:08Z

@wenh06 can you fix the formatting errors / then the pytest errors? thanks

These errors are caused by recent updates of setuptools_scm, see this issue and another issue. The latest version might have fixed this. Try another merge and it might work now.

wenh06 · 2021-09-04T15:03:12Z

@wenh06 can you fix the formatting errors / then the pytest errors? thanks

Almost all errors happened during installing dependencies as follows

Collecting black==20.8b1
  Downloading black-20.8b1.tar.gz (1.1 MB)
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'done'
    Preparing wheel metadata: started
    Preparing wheel metadata: finished with status 'error'
    ERROR: Command errored out with exit status 1:
     command: /opt/hostedtoolcache/Python/3.8.11/x64/bin/python /opt/hostedtoolcache/Python/3.8.11/x64/lib/python3.8/site-packages/pip/_vendor/pep517/in_process/_in_process.py prepare_metadata_for_build_wheel /tmp/tmprk_1nyhe
         cwd: /tmp/pip-install-uksyxpdr/black_9e371aff060b4f01a802c90b27fa15f8
    Complete output (30 lines):
    Traceback (most recent call last):
      File "/opt/hostedtoolcache/Python/3.8.11/x64/lib/python3.8/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 349, in <module>
        main()
      File "/opt/hostedtoolcache/Python/3.8.11/x64/lib/python3.8/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 331, in main
        json_out['return_val'] = hook(**hook_input['kwargs'])
      File "/opt/hostedtoolcache/Python/3.8.11/x64/lib/python3.8/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 151, in prepare_metadata_for_build_wheel
        return hook(metadata_directory, config_settings)
      File "/tmp/pip-build-env-6h_d7wkj/overlay/lib/python3.8/site-packages/setuptools/build_meta.py", line 166, in prepare_metadata_for_build_wheel
        self.run_setup()
      File "/tmp/pip-build-env-6h_d7wkj/overlay/lib/python3.8/site-packages/setuptools/build_meta.py", line 150, in run_setup
        exec(compile(code, __file__, 'exec'), locals())
      File "setup.py", line 48, in <module>
        setup(
      File "/tmp/pip-build-env-6h_d7wkj/overlay/lib/python3.8/site-packages/setuptools/__init__.py", line 153, in setup
        return distutils.core.setup(**attrs)
      File "/opt/hostedtoolcache/Python/3.8.11/x64/lib/python3.8/distutils/core.py", line 108, in setup
        _setup_distribution = dist = klass(attrs)
      File "/tmp/pip-build-env-6h_d7wkj/overlay/lib/python3.8/site-packages/setuptools/dist.py", line 455, in __init__
        _Distribution.__init__(self, {
      File "/opt/hostedtoolcache/Python/3.8.11/x64/lib/python3.8/distutils/dist.py", line 292, in __init__
        self.finalize_options()
      File "/tmp/pip-build-env-6h_d7wkj/overlay/lib/python3.8/site-packages/setuptools/dist.py", line 807, in finalize_options
        ep(self)
      File "/tmp/pip-build-env-6h_d7wkj/overlay/lib/python3.8/site-packages/setuptools_scm/integration.py", line 90, in infer_version
        config = Configuration.from_file(dist_name=dist_name)
      File "/tmp/pip-build-env-6h_d7wkj/overlay/lib/python3.8/site-packages/setuptools_scm/config.py", line 181, in from_file
        defn = _load_toml(data)
      File "/tmp/pip-build-env-6h_d7wkj/overlay/lib/python3.8/site-packages/setuptools_scm/config.py", line 53, in _lazy_tomli_load
        from tomli import loads
    ModuleNotFoundError: No module named 'tomli'

sanchit97 · 2021-09-09T20:44:26Z

Looks like the workflow is still loading in the broken black package.
One simple fix would be to create a new PR and discard this one because Actions would be forced to re-run from scratch. (The issue has been fixed from their end)

qiyanjun · 2021-09-09T20:50:06Z

Let me rerun the workflow

…

Sent from my iPhone

On Sep 9, 2021, at 16:44, Sanchit Sinha ***@***.***> wrote: Looks like the workflow is still loading in the broken black package. One simple fix would be to create a new PR and discard this one because Actions would be forced to re-run from scratch. (The issue has been fixed from their end) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

wenh06 · 2021-09-11T03:07:09Z

Looks like the workflow is still loading in the broken black package.
One simple fix would be to create a new PR and discard this one because Actions would be forced to re-run from scratch. (The issue has been fixed from their end)

May I can add several commits, since I found some other typos.

there's no member fout in the CSVLogger, and there's no fout assigned to instances of CSVLogger elsewhere in Textattack

…dText QData#526

wenh06 · 2021-09-22T02:43:57Z

I fixed #526 here @jxmorris12

jxmorris12 · 2021-09-22T13:21:56Z

Thanks @wenh06 - LGTM! running the tests now

qiyanjun · 2021-09-22T18:43:47Z

@Hanyu-Liu-123 mind helping me check this for one more time?

Hanyu-Liu-123 · 2021-09-22T21:10:11Z

@Hanyu-Liu-123 mind helping me check this for one more time?

no problem!

wenh06 and others added 6 commits August 3, 2021 22:27

add DownloadConfig in datasets/huggingface_dataset.py

ff397ef

fix incorrect __eq__ method of AttackedText in textattack/shared/atta…

f17c297

…cked_text.py

Merge branch 'wenh06'

7a7d211

Update huggingface_dataset.py

6beca91

minor format issue

fix bug in method of in

ac2cea8

Merge branch 'wenh06'

19e253a

qiyanjun requested a review from jinyongyoo August 21, 2021 18:24

revert changes made in huggingface_dataset.py

3a9551d

download config was added in previous 2 commits, which is useless for most of the people, hence is removed.

Merge branch 'QData:master' into master

b88fbdb

fix typos

ac06346

wenh06 added 2 commits September 11, 2021 11:15

update the close method of CSVLogger

fc313f4

there's no member fout in the CSVLogger, and there's no fout assigned to instances of CSVLogger elsewhere in Textattack

fix typos

630d5b5

wenh06 mentioned this pull request Sep 21, 2021

method _text_index_of_word_index of AttackedText has bugs #526

Closed

wenh06 added 3 commits September 22, 2021 10:34

Merge remote-tracking branch 'upstream/master'

b3530d9

fix bugs in the method _text_index_of_word_index in the class Attacke…

d7afffc

…dText QData#526

Merge branch 'master' into wenh06

a857fde

fix typos in _text_index_of_word_index of AttackedText

30a5ead

Merge branch 'wenh06'

2c5dc2c

black format fix

2d7daa6

qiyanjun requested review from jxmorris12 and Hanyu-Liu-123 and removed request for jinyongyoo September 22, 2021 19:50

jxmorris12 approved these changes Sep 22, 2021

View reviewed changes

jxmorris12 merged commit cdfcba4 into QData:master Sep 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix incorrect `eq` method of `AttackedText` in `textattack/shared/attacked_text.py` #509

Fix incorrect `eq` method of `AttackedText` in `textattack/shared/attacked_text.py` #509

wenh06 commented Aug 3, 2021 •

edited by qiyanjun

Loading

qiyanjun commented Aug 6, 2021

qiyanjun commented Aug 21, 2021

wenh06 commented Aug 22, 2021

sanchit97 commented Sep 2, 2021

wenh06 commented Sep 3, 2021

sanchit97 commented Sep 3, 2021

qiyanjun commented Sep 3, 2021

wenh06 commented Sep 4, 2021

wenh06 commented Sep 4, 2021

sanchit97 commented Sep 9, 2021

qiyanjun commented Sep 9, 2021 via email

wenh06 commented Sep 11, 2021

wenh06 commented Sep 22, 2021

jxmorris12 commented Sep 22, 2021

qiyanjun commented Sep 22, 2021

Hanyu-Liu-123 commented Sep 22, 2021

Fix incorrect __eq__ method of AttackedText in textattack/shared/attacked_text.py #509

Fix incorrect __eq__ method of AttackedText in textattack/shared/attacked_text.py #509

Conversation

wenh06 commented Aug 3, 2021 • edited by qiyanjun Loading

What does this PR do?

Summary

Additions

Changes

Deletions

Checklist

qiyanjun commented Aug 6, 2021

qiyanjun commented Aug 21, 2021

wenh06 commented Aug 22, 2021

sanchit97 commented Sep 2, 2021

wenh06 commented Sep 3, 2021

sanchit97 commented Sep 3, 2021

qiyanjun commented Sep 3, 2021

wenh06 commented Sep 4, 2021

wenh06 commented Sep 4, 2021

sanchit97 commented Sep 9, 2021

qiyanjun commented Sep 9, 2021 via email

wenh06 commented Sep 11, 2021

wenh06 commented Sep 22, 2021

jxmorris12 commented Sep 22, 2021

qiyanjun commented Sep 22, 2021

Hanyu-Liu-123 commented Sep 22, 2021

Fix incorrect `eq` method of `AttackedText` in `textattack/shared/attacked_text.py` #509

Fix incorrect `eq` method of `AttackedText` in `textattack/shared/attacked_text.py` #509

wenh06 commented Aug 3, 2021 •

edited by qiyanjun

Loading