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.

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

Remove argparse from requirements #1202

Merged
merged 4 commits into from May 14, 2018
Merged

Conversation

@nelson-liu
Copy link
Member

nelson-liu commented May 12, 2018

Since allennlp is python 3.x only, it doesn't really make sense to have argparse in the requirements --- in fact, having argparse in the requirements seems to break conda (since conda install argparse only works for python 2.x)

I suspect that this was kept from a copy-paste from deep_qa and just never changed, but let me know if there's a reason for keeping argparse around.

@nelson-liu nelson-liu requested review from schmmd and DeNeutoy May 12, 2018
@schmmd

This comment has been minimized.

Copy link
Member

schmmd commented May 14, 2018

@nelson-liu don't we need argparse for imports such as https://github.com/allenai/allennlp/blob/master/scripts/ai2-internal/run_with_beaker.py#L5 and https://github.com/allenai/allennlp/blob/master/allennlp/commands/elmo.py#L54? Since the continuous build passed I'm probably missing something since I would have expected a build failure without argparse.

@joelgrus

This comment has been minimized.

Copy link
Collaborator

joelgrus commented May 14, 2018

argparse is part of the standard library since Python 2.7, the pip installable version is only needed if you're using 2.6 or earlier

@schmmd
schmmd approved these changes May 14, 2018
@schmmd

This comment has been minimized.

Copy link
Member

schmmd commented May 14, 2018

Great!

nelson-liu added 3 commits May 14, 2018
@nelson-liu nelson-liu merged commit 10e7e99 into allenai:master May 14, 2018
3 checks passed
3 checks passed
Pull Requests (AllenNLP) TeamCity build finished
Details
codecov/patch Coverage not affected when comparing e1fdad6...9406cb6
Details
codecov/project 92% remains the same compared to e1fdad6
Details
@nelson-liu nelson-liu deleted the nelson-liu:remove_argparse branch May 14, 2018
gabrielStanovsky pushed a commit to gabrielStanovsky/allennlp that referenced this pull request Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.