Add regex to "install_requires"

[seemethere]

Fixes #1951

[osalpekar]

Thanks! I'm wondering how this escaped smoke tests?

[malfet]

LGTM, but can we please get an ack from torchtext team that it is indeed the case. Also, why isn't there a torchdata dependency?

[joecummings]

The dependency on regex is relatively recent, but yes I'm also confused as to how this escaped smoke tests. Looking into this now.

Also, it looks like you might need to add regex as a dependency here, as well:

text/.circleci/unittest/windows/scripts/environment.yml

Line 8 in 5eb33ce

- pip:

CC @Nayef211 for question about Torchdata.

[seemethere]

The dependency on regex is relatively recent, but yes I'm also confused as to how this escaped smoke tests. Looking into this now.

Also, it looks like you might need to add regex as a dependency here, as well:

text/.circleci/unittest/windows/scripts/environment.yml

Line 8 in 5eb33ce

- pip:

CC @Nayef211 for question about Torchdata.

I'm pretty sure regex gets installed as a transitive dependency for the unittest workflows which might explain the discrepancy. Also the binary builds don't run smoke tests (like importing torchtext) so it's understandable how this wasn't caught.

[seemethere]

As well, why do we even need this dependency in the first place? Is the built-in re module not good enough for the usecase of findall?

cc @rshraga

[Nayef211]

As well, why do we even need this dependency in the first place? Is the built-in re module not good enough for the usecase of findall?

cc @rshraga

I'm not 100% sure why we didn't use re but you can find some relevant discussions around adding using regex in the internal diff thread . IIUC this is a temporary addition and will be removed as soon as @rshraga migrates the regex pattern matching logic to a C++ operator.

Also, why isn't there a torchdata dependency?

@malfet see #1702 where we tagged you guys in asking what the process would be to make torchdata a required dep. I believe there was also an internal chat thread about this but I'm not sure there was a resolution.

[seemethere]

As well, why do we even need this dependency in the first place? Is the built-in re module not good enough for the usecase of findall?
cc @rshraga

I'm not 100% sure why we didn't use re but you can find some relevant discussions around adding using regex in the internal diff thread . IIUC this is a temporary addition and will be removed as soon as @rshraga migrates the regex pattern matching logic to a C++ operator.

Also, why isn't there a torchdata dependency?

@malfet see #1702 where we tagged you guys in asking what the process would be to make torchdata a required dep. I believe there was also an internal chat thread about this but I'm not sure there was a resolution.

Actually switching it to re provides no negative results so I'm just going to submit a new PR to do that instead

[seemethere]

Closing in favor of #1953