search

Origen-SDK / o2

MIT License
4 stars 0 forks source link

added in custom error types #64

Closed info-rchitect closed 4 years ago

info-rchitect commented 4 years ago

I think having some common error types will improve our error messaging over time. We didn't really do this in O1 and I think it is highly encouraged in general. Let me know if you disagree.

ginty commented 4 years ago

Looks good.

Wonder if we should namespace these? e.g. OrigenDuplicateError, OrigenNotFoundError, etc. ?

info-rchitect commented 4 years ago

Looks good. Wonder if we should namespace these? e.g. OrigenDuplicateError, OrigenNotFoundError, etc. ?

I would be fine with that, let's see if anyone else has any concerns.

coreyeng commented 4 years ago

I think they're already kind of namespaced by being in the origen module. importing would end up looking like from origen... import OrigenError, which may be a bit redundant.

coreyeng commented 4 years ago

You can always do an import as ... if there's conflicts in a particular case as well.

ginty commented 4 years ago

Yeah good point, I'm still thinking in Ruby terms I guess.

ginty commented 4 years ago

Thanks for the update to the error name, I don't see the logic behind the sub_blocks change though (and a shame you coupled it this PR).

ginty commented 4 years ago

Sorry, I do see why you did it now, but this is going down a path I don't really like. Rust is the source of authority on what's in the model and should be bubbling up errors rather than adding an additional layer on top in Py.

ginty commented 4 years ago

In fact, it should already be raising an error in this case, does it not?

https://github.com/Origen-SDK/o2/blob/master/rust/origen/src/core/dut.rs#L204

info-rchitect commented 4 years ago

Sorry, I do see why you did it now, but this is going down a path I don't really like. Rust is the source of authority on what's in the model and should be bubbling up errors rather than adding an additional layer on top in Py.

If you don't have sub_blocks in Python, how can you iterate?

ginty commented 4 years ago

I think if you try dut.sub_blocks you will find that it does return a dict containing the sub_blocks which you can iterate on.

info-rchitect commented 4 years ago

I think if you try dut.sub_blocks you will find that it does return a dict containing the sub_blocks which you can iterate on.

I got an error, am I missing something wrt the API?

    def test_duplicate_instance_error():
        boot_falcon()
        assert list(origen.dut.sub_blocks.keys()) == ['core0', 'core1', 'core2', 'core3']
>       for sub_block in origen.dut.sub_blocks():
E       TypeError: 'Proxy' object is not callable

tests/errors_test.py:11: TypeError
info-rchitect commented 4 years ago

@ginty last commit removes the sub block edits, we can figure out direction in another PR.