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

parse arguments from dict #4869

Merged

Conversation

patil-suraj
Copy link
Contributor

This PR adds parse_dict method to HfArgumentParser to allow parsing arguments from dict

@julien-c
As suggested by you here #4791, I've added a simple unit test to check if the dataclass returned by parse_dict is same as manually initialised one.

@codecov
Copy link

codecov bot commented Jun 9, 2020

Codecov Report

Merging #4869 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4869      +/-   ##
==========================================
+ Coverage   76.55%   76.57%   +0.01%     
==========================================
  Files         128      128              
  Lines       21502    21510       +8     
==========================================
+ Hits        16461    16471      +10     
+ Misses       5041     5039       -2     
Impacted Files Coverage Δ
src/transformers/hf_argparser.py 69.23% <100.00%> (+2.96%) ⬆️
src/transformers/modeling_tf_utils.py 87.12% <0.00%> (-0.48%) ⬇️
src/transformers/file_utils.py 74.27% <0.00%> (+0.41%) ⬆️
src/transformers/trainer.py 39.62% <0.00%> (+0.94%) ⬆️

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 9f5d5a5...4461f41. Read the comment docs.

@@ -152,6 +152,20 @@ def test_with_optional(self):
args = parser.parse_args("--foo 12 --bar 3.14 --baz 42 --ces a b c --des 1 2 3".split())
self.assertEqual(args, Namespace(foo=12, bar=3.14, baz="42", ces=["a", "b", "c"], des=[1, 2, 3]))

def test_parse_dict(self):
parser = HfArgumentParser(BasicExample)
Copy link
Member

Choose a reason for hiding this comment

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

This is nit-picking, but the unit test would be more realistic if it took more than one dataclass, like ((BasicExample, AnotherExample))

@patil-suraj
Copy link
Contributor Author

Hi @LysandreJik , what do you think about this ? If it's not really necessary, I will close the PR. Thanks!

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

No, I think this is a cool addition, it just got off my radar. Thanks!

@LysandreJik LysandreJik merged commit 838dc06 into huggingface:master Jul 31, 2020
Mehrad0711 pushed a commit to Mehrad0711/transformers that referenced this pull request Aug 3, 2020
* add parse_dict to parse arguments from dict

* add unit test for parse_dict
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants