Skip to content
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

502 remove support for xls files #505

Merged
merged 6 commits into from Mar 2, 2021
Merged

502 remove support for xls files #505

merged 6 commits into from Mar 2, 2021

Conversation

aloukina
Copy link
Collaborator

@aloukina aloukina commented Mar 1, 2021 •
edited by desilinguist

This PR closes #502.

  • Removed support for xls files
  • Added note to the documentation to highlight this

desilinguist
desilinguist requested changes Mar 1, 2021
View reviewed changes
Copy link
Member

@desilinguist desilinguist left a comment

Choose a reason for hiding this comment

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

Looks great! Just some minor comments/suggestions. Also I see that you added lr_rsmtool_rsmpredict.model file? Was that an accident?

rsmtool/reader.py Outdated Show resolved Hide resolved
tests/test_reader.py Show resolved Hide resolved
doc/pipeline.rst Show resolved Hide resolved
@aloukina @desilinguist
Update rsmtool/reader.py
b11734c 
Co-authored-by: Nitin Madnani <nmadnani@ets.org>
@coveralls
Copy link

coveralls commented Mar 1, 2021 •
edited

Coverage Status

Coverage remained the same at 93.074% when pulling f275d90 on 502-remove-support-for-xls-files into 3698033 on main.

@aloukina
Copy link
Collaborator Author

aloukina commented Mar 1, 2021

I removed the .model file: I am puzzled why it was added...

mulhod
mulhod approved these changes Mar 2, 2021
View reviewed changes
Copy link
Contributor

@mulhod mulhod left a comment

Choose a reason for hiding this comment

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

LGTM!

@desilinguist desilinguist merged commit 08291b7 into main Mar 2, 2021
@delete-merged-branch delete-merged-branch bot deleted the 502-remove-support-for-xls-files branch March 2, 2021 15:46
srhrshr pushed a commit to srhrshr/rsmtool that referenced this pull request Oct 23, 2021
@desilinguist
Merge pull request EducationalTestingService#505 from EducationalTest…
109b7f3 
…ingService/502-remove-support-for-xls-files

502 remove support for xls files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@desilinguist desilinguist desilinguist approved these changes

@mulhod mulhod mulhod approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove support for xls files
4 participants
@aloukina @coveralls @desilinguist @mulhod