Skip to content

clean-up stale code #1654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Mar 13, 2022
Merged

clean-up stale code #1654

merged 3 commits into from
Mar 13, 2022

Conversation

parmeet
Copy link
Contributor

@parmeet parmeet commented Mar 12, 2022 •
edited
Loading

Summary:

  • Removed stale code that is no longer used in the library due to changes in documentation decorators and Datasets migration
  • Removed unicode_csv_reader that is meant for python 2

@codecov
Copy link

codecov bot commented Mar 12, 2022 •
edited
Loading

Codecov Report

Merging #1654 (a9e68c6) into main (1ce1fab) will increase coverage by 2.47%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1654      +/-   ##
==========================================
+ Coverage   86.14%   88.62%   +2.47%     
==========================================
  Files          51       51              
  Lines        2374     2276      -98     
==========================================
- Hits         2045     2017      -28     
+ Misses        329      259      -70
Impacted Files Coverage Δ
torchtext/data/datasets_utils.py 88.19% <ø> (+25.80%) ⬆️
torchtext/utils.py 80.17% <ø> (-0.89%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ce1fab...a9e68c6. Read the comment docs.

@parmeet parmeet changed the title [WIP] clean-up stale code clean-up stale code Mar 12, 2022
@parmeet
Copy link
Contributor Author

parmeet commented Mar 12, 2022

@VirgileHlav looks like removing stale code helps increase the coverage :)

@parmeet parmeet merged commit 8b08b87 into pytorch:main Mar 13, 2022
@parmeet parmeet deleted the remove_stale_utilities branch March 13, 2022 13:48
@xuzhao9 xuzhao9 mentioned this pull request Mar 14, 2022
Open
facebook-github-bot pushed a commit to facebookresearch/pytext that referenced this pull request Mar 16, 2022
@abhinavarora @facebook-github-bot
Replace unicode csv reader from legacy pytext code as fbcode is migra…
b6e8e8a 
…ted to Python3

Summary: Since we deprecated `unicode_csv_reader` in [OSS](pytorch/text#1654), this diff removes internale references to it by replacing `unicode_csv_reader` with Python's `csv.reader`. In Python3 this produces the correct behavior and fbcode no longer has Python2.

Reviewed By: parmeet

Differential Revision: D34903748

fbshipit-source-id: bee0c95244f38ea23395b6f984ea03da8d50f187
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nayef211 Nayef211 Nayef211 approved these changes

@abhinavarora abhinavarora Awaiting requested review from abhinavarora

@vcm2114 vcm2114 Awaiting requested review from vcm2114

Assignees
No one assigned
Labels
ciflow/default cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@parmeet @Nayef211 @facebook-github-bot