- Notifications
You must be signed in to change notification settings - Fork1.3k
Implementmapping trait forGenericAlias#3374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
vm/src/builtins/genericalias.rs Outdated
| for idx in 0..tuple.len() { | ||
| if tuple.fast_getitem(idx).is(item) { | ||
| return Some(idx); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
PyTuple is immutable so it doesn't have a lock.
| for idxin0..tuple.len(){ | |
| if tuple.fast_getitem(idx).is(item){ | |
| returnSome(idx); | |
| } | |
| } | |
| params.as_slice().position(|p| p.is(arg)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ah.. I agree with that. Thanks!
vm/src/builtins/genericalias.rs Outdated
| @@ -1,10 +1,12 @@ | |||
| use crate::{ | |||
| builtins::tuple::IntoPyTuple, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is not intended to be used as an API. I'd better to hide this better than now.vm.ctx.new_tuple orPyTuple::new_ref will suggest more consistent way like other types.
vm/src/builtins/genericalias.rs Outdated
| } | ||
| } | ||
| let newargs = newargs.into_pytuple(vm); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| let newargs = newargs.into_pytuple(vm); | |
| let newargs = newargs.into(); |
Into is enough to convert from PyRef to PyObjectRef
0cf8b3a to9870276Comparevm/src/builtins/genericalias.rs Outdated
| if let Ok(sub_params) = PyTupleRef::try_from_object(vm, sub_params) { | ||
| let sub_args = sub_params | ||
| .as_slice() | ||
| .iter() | ||
| .map(|arg| { | ||
| if let Some(idx) = tuple_index(params, arg) { | ||
| argitems[idx].clone() | ||
| } else { | ||
| arg.clone() | ||
| } | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
| let sub_args: PyObjectRef = PyTuple::new_ref(sub_args, &vm.ctx).into(); | ||
| Some(obj.get_item(sub_args, vm)) | ||
| } else { | ||
| None | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| ifletOk(sub_params) =PyTupleRef::try_from_object(vm, sub_params){ | |
| let sub_args = sub_params | |
| .as_slice() | |
| .iter() | |
| .map(|arg|{ | |
| ifletSome(idx) =tuple_index(params, arg){ | |
| argitems[idx].clone() | |
| }else{ | |
| arg.clone() | |
| } | |
| }) | |
| .collect::<Vec<_>>(); | |
| let sub_args:PyObjectRef =PyTuple::new_ref(sub_args,&vm.ctx).into(); | |
| Some(obj.get_item(sub_args, vm)) | |
| }else{ | |
| None | |
| } | |
| PyTupleRef::try_from_object(vm, sub_params).ok().map(|sub_params|{ | |
| let sub_args = sub_params | |
| .as_slice() | |
| .iter() | |
| .map(|arg|{ | |
| ifletSome(idx) =tuple_index(params, arg){ | |
| argitems[idx].clone() | |
| }else{ | |
| arg.clone() | |
| } | |
| }) | |
| .collect::<Vec<_>>(); | |
| let sub_args:PyObjectRef =PyTuple::new_ref(sub_args,&vm.ctx).into(); | |
| Some(obj.get_item(sub_args, vm)) | |
| }) |
vm/src/builtins/genericalias.rs Outdated
| let mut new_args: Vec<PyObjectRef> = Vec::with_capacity(zelf.args.len()); | ||
| for arg in zelf.args.as_slice().iter() { | ||
| if is_typevar(arg) { | ||
| let idx = tuple_index(&zelf.parameters, arg).unwrap(); | ||
| new_args.push(arg_items[idx].clone()); | ||
| } else { | ||
| new_args.push(subs_tvars(arg.clone(), &zelf.parameters, arg_items, vm)?); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| letmut new_args:Vec<PyObjectRef> =Vec::with_capacity(zelf.args.len()); | |
| for argin zelf.args.as_slice().iter(){ | |
| ifis_typevar(arg){ | |
| let idx =tuple_index(&zelf.parameters, arg).unwrap(); | |
| new_args.push(arg_items[idx].clone()); | |
| }else{ | |
| new_args.push(subs_tvars(arg.clone(),&zelf.parameters, arg_items, vm)?); | |
| } | |
| } | |
| let new_args = zelf.args.as_slice().iter().map(|arg|{ | |
| ifis_typevar(arg){ | |
| let idx =tuple_index(&zelf.parameters, arg).unwrap(); | |
| Ok(arg_items[idx].clone()) | |
| }else{ | |
| subs_tvars(arg.clone(),&zelf.parameters, arg_items, vm) | |
| } | |
| }).collect::<Result<Vec<_>,_>>()?; |
with_capacity is inferred through iterator
SnowaprilOct 26, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
wow..collect::<Result<Vec<_>, _>> seems great. I didn't know thatVec<Result> can be converted like that
9870276 to4705b04CompareSigned-off-by: snowapril <sinjihng@gmail.com>
Signed-off-by: snowapril <sinjihng@gmail.com>
Signed-off-by: snowapril <sinjihng@gmail.com>
4705b04 to795bfebCompareI add extra test that didn't caught by unittest fromtypingimportTypeVarY=TypeVar('Y')assertdict[str,Y][int]==dict[str,int] |
Thanks! |
Uh oh!
There was an error while loading.Please reload this page.
This revision implements
mappingtrait forGenericAliasAlthough this is not included in#3244, actually
GenericAliasprovide mapping protocol interface.(https://github.com/python/cpython/blob/main/Objects/genericaliasobject.c#L376)
Reference