Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

python 3.8 parser fix on args_that_override #4507

Merged
merged 2 commits into from
Apr 28, 2022
Merged

Conversation

jxmsML
Copy link
Contributor

@jxmsML jxmsML commented Apr 20, 2022

Patch description

I'm running into an issue where the opt['override'] doesn't contain all the arguments in command lines, e.g. when running -m XXX , the key 'model' isn't in opt['override'].

My understanding is it's due to single dash isn't handled properly at the _process_args_to_opts. The option _strings_dict in python>=3.8 has all the doubledashes and no singledash
{....'--model': 'model', '--m': 'model', '--model-file': 'model_file', '--mf': 'model_file', '--im': 'init_model', ...}
however the args_that_override has both single and double dashes.

Notice that unlike _handle_single_dash_addarg where the input consists of only args (e.g. args = ('--help', '-h')) and output reordered args, in _handle_single_dash_parsearg the input consists of args and values (e.g. args = ['i', '--mf', 'zoo:blenderbot2/blenderbot2_400M/model', '--search-server', 'devfair0169:3005/search_server', '--knowledge-access-method', 'classify', '--query-generator-model-file', 'zoo:sea/bart_sq_gen/model', '--m', '....agents', ...] ), and therefore shouldn't be reordered

Testing steps

Other information

Copy link
Contributor

@stephenroller stephenroller left a comment

Choose a reason for hiding this comment

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

approved conditional on tests

@jxmsML jxmsML marked this pull request as draft April 20, 2022 17:52
@jxmsML jxmsML requested a review from stephenroller April 20, 2022 18:51
@jxmsML jxmsML self-assigned this Apr 20, 2022
@jxmsML jxmsML marked this pull request as ready for review April 20, 2022 18:51
@jxmsML jxmsML removed their assignment Apr 20, 2022
@jxmsML jxmsML merged commit 4291c8a into main Apr 28, 2022
@jxmsML jxmsML deleted the override_singledash branch April 28, 2022 16:40
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