Hi, I want to work on this issue.

Assigned it to you! Let me know if you need help

Can I try to work on this or any other "gud first issue"? The others I saw had pending pull requests that were not resolved.

Can I try to work on this or any other "gud first issue"? The others I saw had pending pull requests that were not resolved.

Hi, I am already working on this.
You can try other good first issue.

Can I try to work on this or any other "gud first issue"? The others I saw had pending pull requests that were not resolved.

Hi @narang99,

Thanks for offering to contribute! I just marked a few more issues as "good firs issue", feel free look through them and comment on one or two that you find interesting. Happy to discuss them with you if needed.

Hi @gmagogsfm,

I'm new to pytorch, so was going through a couple of "good first issue" labels to see if I can work on some of them. More specifically for this issue, I noticed the use of py::getattr without the defaults and also a comment related to hasattr for python modules here. Is there a reason for hasattr not being implemented using py::hasattr and not using py::getattr with default for python modules in general?

Hi @gmagogsfm,

I'm new to pytorch, so was going through a couple of "good first issue" labels to see if I can work on some of them. More specifically for this issue, I noticed the use of py::getattr without the defaults and also a comment related to hasattr for python modules here. Is there a reason for hasattr not being implemented using py::hasattr and not using py::getattr with default for python modules in general?

Hi @aaditya-panik,

Thanks for the question. TorchScript compiles a PyTorch program into an archive, which can be loaded into Python-free environment for execution, so py::getattr can not be used. As for why current implementation of getattr doesn't support default value, it is because it wasn't needed before, so it was not implemented yet.

I am working on this, as a way to get myself familiar with TorchScript codebase.

So my plan for fixing this is to first verify that the number of args is 2 or 3 and then replace this line:
return obj->attr(apply.range(), method, name);

so that the IR returned is some sort of IF expression, essentially:

attribute not found with the given name ? default val param for attr : obj->attr(apply.range(), method, name)

Is this the right plan? Please comment.

I too thought along these lines. Here's what I could manage to write. Been busy with other work to validate if this is the way to go

https://github.com/aaditya-panik/pytorch/blob/a4de74a219c0e40610e2147ac2e48ec367b15f8b/torch/csrc/jit/frontend/ir_emitter.cpp#L2871-L2907

actually, the right syntax tree to emit is a prim::If node with two blocks, one for True one for False

something like this:

`
def g(x):
a = T()
return 1 if a.my == "HI" else 2

Graph:

graph(%x : Tensor):
%5 : str = prim::Constantvalue="HI" # ts1.py:13:21
%8 : int = prim::Constantvalue=1 # ts1.py:13:8
%9 : int = prim::Constantvalue=2 # ts1.py:13:31
%a.1 : torch.T = prim::CreateObject()
%2 : NoneType = prim::CallMethodname="init" # ts1.py:12:5
%my : str = prim::GetAttrname="my"
%6 : bool = aten::eq(%my, %5) # ts1.py:13:13
%10 : int = prim::If(%6) # ts1.py:13:8
block0():
-> (%8)
block1():
-> (%9)
return (%10)
`