Panic vs Error

[alimoeeny]

Hi @galeone :)
And as always, thank you for the hard work and for keeping the lights on.

I was wondering if you think it is possible to reduce the number of panic calls in this repository?
Would it be okay if I send you PRs that would remove the calls to panic and instead return an error?
If you think that is a reasonable thing to do, I assume we might need a separate develop branch for a while since these will be breaking changes (I mean backward incompatible) due to the changes in the signature of these functions?

One example is

tfgo/image/image.go

Line 74 in d89a5c7

panic(fmt.Errorf("unsupported image extension %s", ext))

where image.Read can return an error if the file extension is not recognized, rather than calling panic

[galeone]

Hi! The style of the library is to use method chaining for building the graph (in your linked code, for example, it returns the updated *Image), and since the graph must be well defined before executing it panic, imho, it makes sense. If you find a way to propagate the error inside the graph definition (removing the panic, and maybe adding an error field inside the various Structure) and add a method Error to invoke at the end of the graph creation, it would also make sense. Let's keep the method chaining approach, but I agree in adding errors instead of panic wherever possible and make sense. (Sorry for the not well formatted response but I'm afk)

…

On Fri, Oct 28, 2022, 13:40 Ali Moeeny ***@***.***> wrote: Hi @galeone <https://github.com/galeone> :) And as always, thank you for the hard work and for keeping the lights on. I was wondering if you think it is possible to reduce the number of panic calls in this repository? Would it be okay if I send you PRs that would remove the calls to panic and instead return an error? If you think that is a reasonable thing to do, I assume we might need a separate develop branch for a while since these will be breaking changes (I mean backward incompatible) due to the changes in the signature of these functions? One example is https://github.com/galeone/tfgo/blob/d89a5c7e31e1d876a46136e30e7891dbc4272b69/image/image.go#L74 where image.Read can return an error if the file extension is not recognized, rather than calling panic — Reply to this email directly, view it on GitHub <#77>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACAJSDDNOHU52XOHQ6XZLCLWFO3RZANCNFSM6AAAAAARQ7TTH4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[alimoeeny]

Thanks @galeone , I see. let me think about it, I am sure there are ways to pass the error along while keeping the chaining.
I don't think any of them are very "idiomatic" though.
Let's think about it and see if we can find a solution that is elegant and practical.

• My immediate reaction was, we can have separate functions / wrappers one set for channing and one set for more idiomatic use, like image.Read is for chaining and image.ReadWithError or image.ReadNP (for No Panic) or image.IRead for idiomatic read :D but this is not an elegant solution.
• then I thought how about passing in a error channel or a pointer to an error so that each function can check if the previous link in the chain has reported any errors. but that is not elegant either

I'll think about it and let you know if I find any good solutions.