Kur mishandles exception when a kurfile requests a nonexistent supplier

go-bro commented 7 years ago

supplier.py:51

        raise ValueError('Ambiguous supplier type in an element of the '
            '"input" list. Exactly one of the following keys must be '
            'present: {}'.format(', '.join(Supplier.get_all_suppliers())))

but Supplier.get_all_suppliers returns a list of types so you get this error:


  File "/Users/go-bro/.virtualenvs/kur/bin/kur", line 11, in <module>
    load_entry_point('kur==0.3.0', 'console_scripts', 'kur')()
  File "/Users/go-bro/.virtualenvs/kur/lib/python3.6/site-packages/kur/__main__.py", line 382, in main
    sys.exit(args.func(args) or 0)
  File "/Users/go-bro/.virtualenvs/kur/lib/python3.6/site-packages/kur/__main__.py", line 61, in train
    func = spec.get_training_function()
  File "/Users/go-bro/.virtualenvs/kur/lib/python3.6/site-packages/kur/kurfile.py", line 259, in get_training_function
    provider = self.get_provider('train')
  File "/Users/go-bro/.virtualenvs/kur/lib/python3.6/site-packages/kur/kurfile.py", line 230, in get_provider
    Supplier.from_specification(x, kurfile=self) for x in supplier_list
  File "/Users/go-bro/.virtualenvs/kur/lib/python3.6/site-packages/kur/kurfile.py", line 230, in <listcomp>
    Supplier.from_specification(x, kurfile=self) for x in supplier_list
  File "/Users/go-bro/.virtualenvs/kur/lib/python3.6/site-packages/kur/supplier/supplier.py", line 50, in from_specification
    ', '.join(Supplier.get_all_suppliers())
TypeError: sequence item 0: expected str instance, type found```

so the error is a bit confusing -- should I submit a PR?

go-bro commented 7 years ago

It looks like both instances of ', '.join(Supplier.get_all_suppliers()) below should be replaced with ', '.join(candidates)

Happy to fork and fix if you guys are open to that 😺


            cls.get_name() for cls in Supplier.get_all_suppliers()
        ) & set(spec.keys())

        if not candidates:
            raise ValueError('Missing the key naming the Supplier type from '
                'an element of the "input" list. Valid keys are: {}'.format(
                ', '.join(Supplier.get_all_suppliers())
            ))
        if len(candidates) > 1:
            raise ValueError('Ambiguous supplier type in an element of the '
                '"input" list. Exactly one of the following keys must be '
                'present: {}'.format(', '.join(Supplier.get_all_suppliers())))```

ajsyp commented 7 years ago

Please feel free to fork and fix! It's pretty simple, but we need to fix both if statements. I think the correct thing is:

        if not candidates:
            raise ValueError('Missing the key naming the Supplier type from '
                'an element of the "input" list. Valid keys are: {}'.format(
                ', '.join(
                    cls.get_name() for cls in Supplier.get_all_suppliers()
                )
            ))
        if len(candidates) > 1:
            raise ValueError('Ambiguous supplier type in an element of the '
                '"input" list. Exactly one of the following keys must be '
                'present: {}'.format(', '.join(candidates)))

I could just patch it, but feel free to fork and submit a PR so that you get credit for it.

go-bro commented 7 years ago

I see, so the second case is where there are too many matches and the first case is where there are no matches.

ajsyp commented 7 years ago

This was fixed by #17

deepgram / kur

Kur mishandles exception when a kurfile requests a nonexistent supplier #15