Skip to content
This repository was archived by the owner on Sep 10, 2025. It is now read-only.

Conversation

@parmeet
Copy link
Contributor

@parmeet parmeet commented Jan 30, 2022

Revert the BC breaking change in the output introduced in #1511

@erip
Copy link
Contributor

erip commented Jan 30, 2022

LGTM! Qq, did tests not catch this?

@parmeet
Copy link
Contributor Author

parmeet commented Jan 31, 2022

LGTM! Qq, did tests not catch this?

ya, they did, I learnt from there only for eg: https://app.circleci.com/pipelines/github/pytorch/text/4693/workflows/447cb44d-4ee2-4361-b954-2eebe700f25d/jobs/157132. Unfortunately, earlier the cache was broken though :(

data_dp = FileOpener(cache_dp, mode="r")
# remove single leading and trailing space from the dataset
return data_dp.readlines(return_path=False).map(lambda t: t.strip())
return data_dp.readlines(return_path=False, strip_newline=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as #1559

@abhinavarora
Copy link
Contributor

LGTM! Qq, did tests not catch this?

ya, they did, I learnt from there only for eg: https://app.circleci.com/pipelines/github/pytorch/text/4693/workflows/447cb44d-4ee2-4361-b954-2eebe700f25d/jobs/157132. Unfortunately, earlier the cache was broken though :(

Wondering why did local testing not catch it?

@parmeet
Copy link
Contributor Author

parmeet commented Feb 3, 2022

closing this PR as this is a reasonable BC breaking change.

@parmeet parmeet closed this Feb 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants