Some error messages are not very helpful

[apragsdale]

For example, an add_pulse that fails will often just say TypeError: pulse[0]: invalid pulse. It would be much more helpful to know why it failed, or at least which field was input improperly.

[grahamgower]

Do you have an example where the error is hard to read? I just checked the error messages folks will get if they try to use an old-style pulse with demes 0.2.0 and they seem ok to me.

# p.py
import demes

b = demes.Builder()
b.add_deme("a", epochs=[dict(start_size=1)])
b.add_deme("b", epochs=[dict(start_size=1)])
b.add_pulse(source="a", dest="b", proportion=0.1, time=100)

$ python p.py
Traceback (most recent call last):
  File "/tmp/p.py", line 6, in <module>
    b.add_pulse(source="a", dest="b", proportion=0.1, time=100)
TypeError: add_pulse() got an unexpected keyword argument 'source'

# p.yaml
time_units: generations
demes:
- name: a
  epochs:
  - start_size: 1
- name: b
  epochs:
  - start_size: 1
pulses:
- {source: a, dest: b, time: 100, proportion: 0.1}

$ demes parse p.yaml
Traceback (most recent call last):
  File "/home/grg/.local/lib/python3.9/site-packages/demes/load_dump.py", line 22, in _open_file_polymorph
    with open(polymorph, mode) as f:
TypeError: expected str, bytes or os.PathLike object, not TextIOWrapper

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/grg/.local/bin/demes", line 8, in <module>
    sys.exit(cli())
  File "/home/grg/.local/lib/python3.9/site-packages/demes/__main__.py", line 212, in cli
    args.func(args)
  File "/home/grg/.local/lib/python3.9/site-packages/demes/__main__.py", line 91, in __call__
    num_documents, graphs = self.load_and_count_documents(args.filename)
  File "/home/grg/.local/lib/python3.9/site-packages/demes/__main__.py", line 130, in load_and_count_documents
    for graph in graph_generator:
  File "/home/grg/.local/lib/python3.9/site-packages/demes/load_dump.py", line 235, in load_all
    yield demes.Graph.fromdict(data)
  File "/home/grg/.local/lib/python3.9/site-packages/demes/demes.py", line 2108, in fromdict
    check_allowed(pulse_data, allowed_fields_pulse, f"pulse[{i}]")
  File "/home/grg/.local/lib/python3.9/site-packages/demes/demes.py", line 138, in check_allowed
    raise KeyError(
KeyError: "pulse[0]: unexpected field: 'source'. Allowed fields are: ['sources', 'dest', 'time', 'proportions']"

Having the _open_file_polymorph bit in the stack trace isn't ideal, but the KeyError looks very reasonable.

But maybe you're thinking of more complex problems?

# p2.yaml
time_units: generations
demes:
- name: a
  epochs:
  - start_size: 1
- name: b
  ancestors: [a]
  start_time: 50
  epochs:
  - start_size: 1
pulses:
- {sources: [a], dest: b, time: 100, proportions: [0.1]}

$ demes parse p2.yaml
Traceback (most recent call last):
  File "/home/grg/.local/lib/python3.9/site-packages/demes/demes.py", line 2114, in fromdict
    graph._add_pulse(
  File "/home/grg/.local/lib/python3.9/site-packages/demes/demes.py", line 1612, in _add_pulse
    self._check_time_intersection(source, dest, time)
  File "/home/grg/.local/lib/python3.9/site-packages/demes/demes.py", line 1513, in _check_time_intersection
    raise ValueError(
ValueError: time 100 not in interval [0, 50], as defined by the time-intersection of a (start_time=inf, end_time=0) and b (start_time=50, end_time=0).

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/grg/.local/bin/demes", line 8, in <module>
    sys.exit(cli())
  File "/home/grg/.local/lib/python3.9/site-packages/demes/__main__.py", line 212, in
cli
    args.func(args)
  File "/home/grg/.local/lib/python3.9/site-packages/demes/__main__.py", line 91, in __call__
    num_documents, graphs = self.load_and_count_documents(args.filename)
  File "/home/grg/.local/lib/python3.9/site-packages/demes/__main__.py", line 130, in
load_and_count_documents
    for graph in graph_generator:
  File "/home/grg/.local/lib/python3.9/site-packages/demes/load_dump.py", line 235, in load_all
    yield demes.Graph.fromdict(data)
  File "/home/grg/.local/lib/python3.9/site-packages/demes/demes.py", line 2121, in fromdict
    raise e.__class__(f"pulse[{i}]: invalid pulse") from e
ValueError: pulse[0]: invalid pulse

The last error is quite terse, but in combination with the earlier error, this is quite informative. Though I admit, it would be nicer if the info was all printed in one exception. Is this what you mean? Any ideas about how we might go about improving this?

[apragsdale]

You're right - most error messages are actually very clear what the problem is. I quickly jotted down this issue with this discussion over in msprime in mind: tskit-dev/msprime#1918

Here, an input graph had a pulse with a list as the dest, instead of a single deme name as a string. For example:

import demes

b = demes.Builder()
b.add_deme("a", epochs=[dict(start_size=1)])
b.add_deme("b", epochs=[dict(start_size=1)])
b.add_pulse(sources=["a"], dest=["b"], proportions=0.1, time=[100])
g = b.resolve()

results in the following, which was not descriptive enough for the user to know what the problem was:

Traceback (most recent call last)
~/Code/demes/demes/demes.py in fromdict(cls, data)
   2113             try:
-> 2114                 graph._add_pulse(
   2115                     sources=pulse_data.pop("sources"),

~/Code/demes/demes/demes.py in _add_pulse(self, sources, dest, proportions, time)
   1608         for deme_name in sources + [dest]:
-> 1609             if deme_name not in self:
   1610                 raise ValueError(f"{deme_name} not in graph")

~/Code/demes/demes/demes.py in __contains__(self, deme_name)
   1306         """
-> 1307         return deme_name in self._deme_map
   1308 

TypeError: unhashable type: 'list'

The above exception was the direct cause of the following exception:

TypeError                                 Traceback (most recent call last)
<ipython-input-11-ab173dfacd33> in <module>
      5 b.add_deme("b", epochs=[dict(start_size=1)])
      6 b.add_pulse(sources=["a"], dest=["b"], proportions=0.1, time=[100])
----> 7 g = b.resolve()

~/Code/demes/demes/demes.py in resolve(self)
   2480         :rtype: Graph
   2481         """
-> 2482         return Graph.fromdict(self.data)
   2483 
   2484     @classmethod

~/Code/demes/demes/demes.py in fromdict(cls, data)
   2119                 )
   2120             except (TypeError, ValueError) as e:
-> 2121                 raise e.__class__(f"pulse[{i}]: invalid pulse") from e
   2122 
   2123         # Sort pulses from oldest to youngest.

TypeError: pulse[0]: invalid pulse

[grahamgower]

That is quite cryptic. We couldy replace all the dict .pop() method calls in Graph.fromdict() with calls to pop_item() so that validation of types occurs earlier.