initial Python support #44

Closed
rvalyi wants to merge 3 commits from rvalyi/mergiraf:add-python into main
First-time contributor

initial Python support. Please consider this might be incomplete or contain mistakes. However I could test it successfully in an Odoo (actually OCA/l10n-brazil) repo as shown in the screenshot. I could later test that the conflict resolutions are correct, which is great.

Thank you so much for Mergiraf, it seems it has a huge potential for us in the OCA Odoo open source community!

initial Python support. Please consider this might be incomplete or contain mistakes. However I could test it successfully in an Odoo (actually OCA/l10n-brazil) repo as shown in the screenshot. I could later test that the conflict resolutions are correct, which is great. Thank you so much for Mergiraf, it seems it has a huge potential for us in the OCA Odoo open source community!
initial Python support
All checks were successful
/ test (pull_request) Successful in 51s
4d911aeadd
wetneb left a comment
Owner

Thank you very much for having a go at it :)

Thank you very much for having a go at it :)
@ -307,0 +310,4 @@
language: tree_sitter_python::LANGUAGE.into(),
atomic_nodes: vec![],
commutative_parents: vec![
CommutativeParent::without_delimiters("module", "\n"),
Owner

Given that Python can have top-level instructions in a module (for instance in a simple Python script, meant to be executed on its own), this is dangerous because it lets the tool reorder instructions whose order matters, no?

I have recently added support for restricting commutativity and I think it would be a good use case here, for instance to only let import statements commute. See the docs here: https://mergiraf.org/adding-a-language.html#adding-children-groups

Given that Python can have top-level instructions in a module (for instance in a simple Python script, meant to be executed on its own), this is dangerous because it lets the tool reorder instructions whose order matters, no? I have recently added support for restricting commutativity and I think it would be a good use case here, for instance to only let import statements commute. See the docs here: https://mergiraf.org/adding-a-language.html#adding-children-groups
Author
First-time contributor

Hello I have seen this and I thought exactly the same. I'm not too sure what the exact expression should be though.

Hello I have seen this and I thought exactly the same. I'm not too sure what the exact expression should be though.
First-time contributor

I use import sorting in all my python project (via ruff format), which would also run via a precommit hock after the merge succeeds...

In my book it's a bug if you rely on module level code, but I guess from a conservative standpoint, order does matter...

I use import sorting in all my python project (via ruff format), which would also run via a precommit hock after the merge succeeds... In my book it's a bug if you rely on module level code, but I guess from a conservative standpoint, order does matter...
@ -307,0 +318,4 @@
signature("class_definition", vec![vec![Field("name")]]),
signature("function_definition", vec![
vec![Field("name")],
vec![Field("parameters")]
Owner

I was under the impression that Python doesn't make it possible to declare two different functions with the same name and different parameter lists, so I would expect that the parameters are not part of the signature. Do you see it differently?

I was under the impression that Python doesn't make it possible to declare two different functions with the same name and different parameter lists, so I would expect that the parameters are not part of the signature. Do you see it differently?
Author
First-time contributor

you are correct, this is a mistake, I'll fix it in a few hours.

you are correct, this is a mistake, I'll fix it in a few hours.
First-time contributor

It's not impossible to do, e.g. I've seen this pattern:


@singledispatch
def do_something(input: Any) -> None
    pass

@do_something.register
def _(input:float) -> None:
    pass
    
@do_something.register
def _(input:str) -> None:
    pass
 

This is valid python code

def func(whatever:Bla):
   pass

# This line could be removed, but actually might make this a "sane" pattern
module.func = func

def func(whatever:Other):
   pass

(it's valid even without the monkey patching: func is simply a variable which got the functon assigned, doesn't matter if the func has a different signature. It's for sure not wise python code and probably all linter will flag that duplication...)

It's not impossible to do, e.g. I've seen this pattern: ```python @singledispatch def do_something(input: Any) -> None pass @do_something.register def _(input:float) -> None: pass @do_something.register def _(input:str) -> None: pass ``` This is valid python code ```python def func(whatever:Bla): pass # This line could be removed, but actually might make this a "sane" pattern module.func = func def func(whatever:Other): pass ``` (it's valid even without the monkey patching: func is simply a variable which got the functon assigned, doesn't matter if the func has a different signature. It's for sure not wise python code and probably all linter will flag that duplication...)
Owner

I've proposed some improvements here: rvalyi/mergiraf#1

I've proposed some improvements here: https://codeberg.org/rvalyi/mergiraf/pulls/1
wetneb approved these changes 2024-11-29 13:15:31 +01:00
wetneb left a comment
Owner

Great, this looks good to merge once the conflicts are resolved (which I can do if you want)

Great, this looks good to merge once the conflicts are resolved (which I can do if you want)
wetneb referenced this pull request from a commit 2024-12-04 09:49:51 +01:00
Owner

I've merged it manually in d57720185b.

I've merged it manually in d57720185bab842e87d48d3d1f3ee8ca076d43cc.
wetneb closed this pull request 2024-12-04 09:50:34 +01:00
All checks were successful
/ test (pull_request) Successful in 37s

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: mergiraf/mergiraf#44
No description provided.