Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List support: append method #1709

Merged
merged 83 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 77 commits
Commits
Show all changes
83 commits
Select commit Hold shift + click to select a range
9e8be09
getting last devel updates into my branch
Farouk-Echaref Jan 26, 2024
82bffa9
feat: List class_def structure & List Methods structre
Farouk-Echaref Jan 26, 2024
a23c504
Merge branch 'devel' into feat/list_support_in_Pyccel
Farouk-Echaref Jan 29, 2024
8ebf4c1
feat: list classdef structure
Farouk-Echaref Jan 30, 2024
4d69c1a
feat: PythonList in type inference
Farouk-Echaref Jan 30, 2024
e270e92
start append class doc
Farouk-Echaref Jan 30, 2024
ea42345
check on List type in classdef
Farouk-Echaref Jan 31, 2024
87c69ed
fixdoc: handle_function() takes type list args not tuple
Farouk-Echaref Jan 31, 2024
0105fb0
append() method basic class
Farouk-Echaref Feb 1, 2024
004df3e
Merge branch 'devel' into feat/list_support_in_Pyccel
Farouk-Echaref Feb 1, 2024
4d4a9a6
feat: semantic support for append() and valid python printer => need …
Farouk-Echaref Feb 1, 2024
6a4cf9a
fix: type check for append() arg
Farouk-Echaref Feb 2, 2024
82c8eeb
fix: raise proper error message in case of incompatibility of types
Farouk-Echaref Feb 2, 2024
d9b16be
fix: missing coma in class_def
Farouk-Echaref Feb 2, 2024
cc0ace0
fix codacy errors
Farouk-Echaref Feb 2, 2024
5472a59
add issue to changlog
Farouk-Echaref Feb 2, 2024
7c3b2bc
fix _print_ListAppend: add newline after each append statement
Farouk-Echaref Feb 2, 2024
3b3ec83
feat: add tests for append list method
Farouk-Echaref Feb 3, 2024
0e595d1
fix: using f-string instead of format() for _print_ListAppend()
Farouk-Echaref Feb 3, 2024
65fff84
fix doc description
Farouk-Echaref Feb 3, 2024
832f2a6
fix condition in get_cls_base to not confuse the check on NativeLists
Farouk-Echaref Feb 3, 2024
6dbe191
fix tests
Farouk-Echaref Feb 3, 2024
561b01e
chore: edits in tests
Farouk-Echaref Feb 3, 2024
d76ebe3
fix: get the correct branch name
Farouk-Echaref Feb 5, 2024
31f65c2
fix condition to check on containers in class_defs
Farouk-Echaref Feb 5, 2024
5ea27e4
fix: get the correct branch name
Farouk-Echaref Feb 5, 2024
607e864
revert to the devel state of .gitignore
Farouk-Echaref Feb 5, 2024
054c8e1
update list append class, this commit will be reverted
Farouk-Echaref Feb 7, 2024
95d7fa4
add new homogeneity checks,and raise an appropriate message
Farouk-Echaref Feb 8, 2024
1a9c2aa
chore: delete import parentheses
Farouk-Echaref Feb 8, 2024
8971fe3
Merge branch 'devel' into List_Support/append_method
Farouk-Echaref Feb 8, 2024
0fe56f1
Merge branch 'devel' of https://github.com/pyccel/pyccel into devel
Farouk-Echaref Feb 8, 2024
ab24e34
Merge branch 'devel' into List_Support/append_method
Farouk-Echaref Feb 8, 2024
13d3348
fix: catch new customized errors
Farouk-Echaref Feb 9, 2024
9a95f60
feat: add new properties for list append method
Farouk-Echaref Feb 9, 2024
20a134a
feat: specify property decorators for the classdef of ListClass
Farouk-Echaref Feb 9, 2024
8e8de64
use new explicit names for the printer
Farouk-Echaref Feb 9, 2024
2dde3ce
chore: add empty line
Farouk-Echaref Feb 9, 2024
fc6faff
chore: update list_functions.py description
Farouk-Echaref Feb 9, 2024
d0b3286
fix codacy prod newline error
Farouk-Echaref Feb 9, 2024
6062a16
fix codacy prod newline error
Farouk-Echaref Feb 9, 2024
6a4e20f
fix codacy prod newline error
Farouk-Echaref Feb 9, 2024
4c17533
fix codacy prod newline error
Farouk-Echaref Feb 9, 2024
3ad7452
feat: add tests for error checking where append is supposed to fail
Farouk-Echaref Feb 9, 2024
0a27c23
fix: assign the correct append arg
Farouk-Echaref Feb 9, 2024
d24f595
disable module docstring
Farouk-Echaref Feb 9, 2024
7f26cc7
codacy delete trailing whitespace
Farouk-Echaref Feb 9, 2024
3421dfe
update list functions docs
Farouk-Echaref Feb 9, 2024
5c4ba6f
fix append tests
Farouk-Echaref Feb 9, 2024
0cbeb0f
fix precision check
Farouk-Echaref Feb 9, 2024
54cc2a4
add error tests
Farouk-Echaref Feb 9, 2024
b0a3369
delete unused import
Farouk-Echaref Feb 9, 2024
d0cfeee
update success and error tests for append method
Farouk-Echaref Feb 12, 2024
ad6fca5
delete redundant check on the containers Tuple/List classes in the se…
Farouk-Echaref Feb 12, 2024
85611b2
update naming in python append printer
Farouk-Echaref Feb 12, 2024
10e2294
delete checks on class_type, List can support different containers
Farouk-Echaref Feb 12, 2024
1c221b9
Merge branch 'devel' of https://github.com/pyccel/pyccel into devel
Farouk-Echaref Feb 12, 2024
44f18ba
Merge branch 'devel' into List_Support/append_method
Farouk-Echaref Feb 12, 2024
89a402a
update test function name
Farouk-Echaref Feb 12, 2024
85ceef6
update docstring of ListAppend
Farouk-Echaref Feb 12, 2024
0197cc1
Merge branch 'devel' into List_Support/append_method
EmilyBourne Feb 12, 2024
93d637a
fix: pass the variable to the printer instead of it's name alone
Farouk-Echaref Feb 12, 2024
8282f57
chore: remove unused import
Farouk-Echaref Feb 12, 2024
e628f6c
Merge branch 'devel' of https://github.com/pyccel/pyccel into devel
Farouk-Echaref Feb 12, 2024
0f39b82
Merge branch 'devel' into List_Support/append_method
Farouk-Echaref Feb 12, 2024
86ca222
revert to a previous state of semantic where tuple check is available
Farouk-Echaref Feb 12, 2024
a0337c3
revert import to devel state
Farouk-Echaref Feb 12, 2024
67ad2fd
Merge branch 'devel' into List_Support/append_method
EmilyBourne Feb 13, 2024
9db04e0
unused decorator property in ListAppend class
Farouk-Echaref Feb 13, 2024
d984ec4
Merge branch 'devel' of https://github.com/pyccel/pyccel into devel
Farouk-Echaref Feb 13, 2024
9b5daa8
Merge branch 'devel' into List_Support/append_method
Farouk-Echaref Feb 13, 2024
09fa662
Merge branch 'devel' of https://github.com/pyccel/pyccel into devel
Farouk-Echaref Feb 13, 2024
8767430
Merge branch 'devel' into List_Support/append_method
Farouk-Echaref Feb 13, 2024
52addad
set the static class attribute _attribute_nodes inside the ListAppend…
Farouk-Echaref Feb 13, 2024
6d94603
Merge branch 'devel' of https://github.com/pyccel/pyccel into devel
Farouk-Echaref Feb 13, 2024
eb9a747
Merge branch 'devel' into List_Support/append_method
Farouk-Echaref Feb 13, 2024
c89bae3
add issue title to the unreleased section
Farouk-Echaref Feb 13, 2024
ce7ec14
rename builtin_objects => builtin_methods/ rename list_functions => l…
Farouk-Echaref Feb 14, 2024
ed6e77c
handle_funcion() takes iterable args
Farouk-Echaref Feb 14, 2024
c349a6e
identical test file
Farouk-Echaref Feb 14, 2024
c16295c
update import in class_def
Farouk-Echaref Feb 14, 2024
0772a46
refactor test file for lists
Farouk-Echaref Feb 14, 2024
2d23899
Add missing __all__ to list_methods.py
yguclu Feb 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ All notable changes to this project will be documented in this file.

### Added

- #1529 : Add Python support for list method `append()`

### Fixed

### Changed
Expand Down
78 changes: 78 additions & 0 deletions pyccel/ast/builtin_objects/list_functions.py
yguclu marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# coding: utf-8
#------------------------------------------------------------------------------------------#
# This file is part of Pyccel which is released under MIT License. See the LICENSE file or #
# go to https://github.com/pyccel/pyccel/blob/master/LICENSE for full license details. #
#------------------------------------------------------------------------------------------#
"""
The List container has a number of built-in methods that are
always available.

This module contains objects which describe these methods within Pyccel's AST.
"""

from pyccel.ast.datatypes import NativeVoid, NativeGeneric, NativeHomogeneousList
from pyccel.ast.internals import PyccelInternalFunction


class ListAppend(PyccelInternalFunction):
"""
Represents a call to the .append() method.

Represents a call to the .append() method of an object with a list type,
which adds an element to the end of the list. This method returns `None`.
The append method is called as follows:

>>> a = [1]
>>> a.append(2)
>>> print(a)
[1, 2]

Parameters
----------
list_variable : Variable
The variable representing the list.

new_elem : Variable
The argument passed to append() method.
"""
__slots__ = ("_list_variable", "_append_arg")
EmilyBourne marked this conversation as resolved.
Show resolved Hide resolved
_attribute_nodes = ("_list_variable", "_append_arg")
_dtype = NativeVoid()
_shape = None
_order = None
_rank = 0
_precision = -1
_class_type = NativeHomogeneousList()
name = 'append'

def __init__(self, list_variable, new_elem) -> None:
conditions = (
new_elem.dtype is not NativeGeneric() and
list_variable.dtype == new_elem.dtype and
list_variable.precision == new_elem.precision and
list_variable.rank - 1 == new_elem.rank
yguclu marked this conversation as resolved.
Show resolved Hide resolved
)
is_homogeneous = list_variable.dtype is not NativeGeneric() and conditions
yguclu marked this conversation as resolved.
Show resolved Hide resolved
if not is_homogeneous:
raise TypeError("Expecting an argument of the same type as the elements of the list")
self._list_variable = list_variable
self._append_arg = new_elem
super().__init__()

@property
def list_variable(self):
"""
Get the variable representing the list.

Get the variable representing the list.
"""
return self._list_variable

@property
def append_argument(self):
"""
Get the argument which is passed to append().

Get the argument which is passed to append().
"""
return self._append_arg
21 changes: 18 additions & 3 deletions pyccel/ast/class_defs.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@
"""
This module contains all types which define a python class which is automatically recognised by pyccel
"""

from pyccel.ast.builtin_objects.list_functions import ListAppend


from .builtins import PythonImag, PythonReal, PythonConjugate
from .core import ClassDef, PyccelFunctionDef
from .c_concepts import CStackArray
from .datatypes import (NativeBool, NativeInteger, NativeFloat,
NativeComplex, NativeString, NativeNumeric,
NativeTuple, CustomDataType)
NativeComplex, NativeString, NativeNumericTypes,
NativeTuple, CustomDataType, NativeHomogeneousList)
from .numpyext import (NumpyShape, NumpySum, NumpyAmin, NumpyAmax,
NumpyImag, NumpyReal, NumpyTranspose,
NumpyConjugate, NumpySize, NumpyResultType,
Expand All @@ -23,6 +27,7 @@
'StringClass',
'NumpyArrayClass',
'TupleClass',
'ListClass',
'literal_classes',
'get_cls_base')

Expand Down Expand Up @@ -130,6 +135,14 @@

#=======================================================================================

ListClass = ClassDef('list', class_type = NativeHomogeneousList(),
methods=[
PyccelFunctionDef('append', func_class = ListAppend,
decorators = {}),
])

#=======================================================================================

TupleClass = ClassDef('tuple', class_type = NativeTuple(),
methods=[
#index
Expand Down Expand Up @@ -217,10 +230,12 @@ def get_cls_base(dtype, precision, container_type):
return None
if precision in (-1, 0, None) and container_type is dtype:
return literal_classes[dtype]
elif dtype in NativeNumeric or isinstance(container_type, NumpyNDArrayType):
elif isinstance(container_type, (*NativeNumericTypes, NumpyNDArrayType)):
return NumpyArrayClass
elif isinstance(container_type, NativeTuple):
return TupleClass
elif isinstance(container_type, NativeHomogeneousList):
return ListClass
else:
if container_type:
type_name = str(container_type)
Expand Down
7 changes: 7 additions & 0 deletions pyccel/codegen/printing/pycode.py
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,13 @@ def _print_NumpyCountNonZero(self, expr):

return "{}({})".format(name, arg)

def _print_ListAppend(self, expr):
method_name = expr.name
list_var = self._print(expr.list_variable)
append_arg = self._print(expr.append_argument)

return f"{list_var}.{method_name}({append_arg})\n"

def _print_Slice(self, expr):
start = self._print(expr.start) if expr.start else ''
stop = self._print(expr.stop) if expr.stop else ''
Expand Down
13 changes: 8 additions & 5 deletions pyccel/parser/semantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1094,7 +1094,7 @@ def _handle_function(self, expr, func, args, is_method = False):
func : FunctionDef instance, Interface instance or PyccelInternalFunction type
The function being called.

args : tuple
args : list
The arguments passed to the function.
yguclu marked this conversation as resolved.
Show resolved Hide resolved

is_method : bool
Expand All @@ -1118,10 +1118,13 @@ def _handle_function(self, expr, func, args, is_method = False):

try:
new_expr = func(*args, **kwargs)
except TypeError:
errors.report(UNRECOGNISED_FUNCTION_CALL,
symbol = expr,
severity = 'fatal')
except TypeError as e:
message = str(e)
if not message:
message = UNRECOGNISED_FUNCTION_CALL
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code isn't tested. Please can you take a look

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/bot accept
The final line is the only one without coverage. It was there before and is a fallback for strange function calls.

errors.report(message,
symbol = expr,
severity = 'fatal')

return new_expr
else:
Expand Down
127 changes: 127 additions & 0 deletions tests/epyccel/test_epyccel_lists.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
# pylint: disable=missing-function-docstring, missing-module-docstring, missing-class-docstring
# coding: utf-8
""" Tests for list methods.
"""

import pytest
from pyccel.epyccel import epyccel

@pytest.mark.parametrize( 'language', [
pytest.param("c", marks = [
pytest.mark.skip(reason="append() not implemented in c"),
pytest.mark.c]),
pytest.param("fortran", marks = [
pytest.mark.skip(reason="append() not implemented in fortran"),
pytest.mark.fortran]),
pytest.param("python", marks = pytest.mark.python)
]
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we not define a language fixture in this file, rather than copying this long decorator many times?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to put xfail marks in a fixture? The xfail marks should be removed in a few weeks or months once the method is supported

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please explain how that would be possible? I was unable to do so since I'm not that familiar with writing test files using pytest so I did in the most basic way

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to put xfail marks in a fixture? The xfail marks should be removed in a few weeks or months once the method is supported

It should be possible:

@pytest.fixture( params=[
        pytest.param("fortran", marks = [
            pytest.mark.skip(reason="list methods not implemented in fortran"),
            pytest.mark.fortran]),
        pytest.param("c", marks = [
            pytest.mark.skip(reason="list methods not implemented in c"),
            pytest.mark.c]),
        pytest.param("python", marks = pytest.mark.python)
    ],
    scope = "module"
)
def language(request):
    return request.param

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you check the refactored version please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did work in my linux machine 🤞

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but I wonder if we should name this fixture something like language_python_ok. At some point we will have some tests in this file which can use the existing language fixture (without the xfails) and some which need this one (and maybe some that need one where only C or only Fortran fails)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. How about using multiple test files and move the tests from one to the other as they start passing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also possible if you have a suggestion for a good name? We may need up to 4 files:

  • Passing in Python only
  • Passing in C and Python
  • Passing in Fortran and Python
  • Passing in C, Fortran and Python

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, using four files is a bit cumbersome... We could call them

  • test_epyccel_lists.py
  • test_epyccel_lists__no_c.py
  • test_epyccel_lists__no_fortran.py
  • test_epyccel_lists__python_only.py

Another solution is using a single file, but providing a utility function which generates the correct decorators with minimum user input.

In any case I suggest that we tackle this in a future PR.

def test_append_basic(language):
def f():
a = [1, 2, 3]
a.append(4)
return a

epyc_f = epyccel(f, language=language)
assert f() == epyc_f()

@pytest.mark.parametrize( 'language', [
pytest.param("c", marks = [
pytest.mark.skip(reason="append() not implemented in c"),
pytest.mark.c]),
pytest.param("fortran", marks = [
pytest.mark.skip(reason="append() not implemented in fortran"),
pytest.mark.fortran]),
pytest.param("python", marks = pytest.mark.python)
]
)
def test_append_multiple(language):
def f():
a = [1, 2, 3]
a.append(4)
a.append(5)
a.append(6)
return a

epyc_f = epyccel(f, language=language)
assert f() == epyc_f()

@pytest.mark.parametrize( 'language', [
pytest.param("c", marks = [
pytest.mark.skip(reason="append() not implemented in c"),
pytest.mark.c]),
pytest.param("fortran", marks = [
pytest.mark.skip(reason="append() not implemented in fortran"),
pytest.mark.fortran]),
pytest.param("python", marks = pytest.mark.python)
]
)
def test_append_list(language):
def f():
a = [[1, 2, 3]]
a.append([4, 5, 6])
return a

epyc_f = epyccel(f, language=language)
assert f() == epyc_f()

@pytest.mark.parametrize( 'language', [
pytest.param("c", marks = [
pytest.mark.skip(reason="append() not implemented in c"),
pytest.mark.c]),
pytest.param("fortran", marks = [
pytest.mark.skip(reason="append() not implemented in fortran"),
pytest.mark.fortran]),
pytest.param("python", marks = pytest.mark.python)
]
)
def test_append_range(language):
def f():
a = [1, 2, 3]
for i in range(0, 1000):
a.append(i)
a.append(1000)
return a

epyc_f = epyccel(f, language=language)
assert f() == epyc_f()

@pytest.mark.parametrize( 'language', [
pytest.param("c", marks = [
pytest.mark.skip(reason="append() not implemented in c"),
pytest.mark.c]),
pytest.param("fortran", marks = [
pytest.mark.skip(reason="append() not implemented in fortran"),
pytest.mark.fortran]),
pytest.param("python", marks = pytest.mark.python)
]
)
def test_append_range_list(language):
def f():
a = [[1, 2, 3]]
for i in range(0, 1000):
a.append([i, i + 1])
return a

epyc_f = epyccel(f, language=language)
assert f() == epyc_f()

@pytest.mark.parametrize( 'language', [
pytest.param("c", marks = [
pytest.mark.skip(reason="append() not implemented in c"),
pytest.mark.c]),
pytest.param("fortran", marks = [
pytest.mark.skip(reason="append() not implemented in fortran"),
pytest.mark.fortran]),
pytest.param("python", marks = pytest.mark.python)
]
)
def test_append_range_tuple(language):
def f():
a = [[1, 2, 3]]
for i in range(0, 1000):
a.append((i, i + 1))
return a

epyc_f = epyccel(f, language=language)
assert f() == epyc_f()
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# pylint: disable=missing-function-docstring, missing-module-docstring

import numpy as np
a = [1,2,3]
b = np.int32(4)
a.append(b)

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# pylint: disable=missing-function-docstring, missing-module-docstring


a = [1,2,3]
a.append([4])

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# pylint: disable=missing-function-docstring, missing-module-docstring

a = [1,2,3]
b = (4,5)
a.append(b)

yguclu marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# pylint: disable=missing-function-docstring, missing-module-docstring

a = [1, 2, 3]
a.append([4])

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# pylint: disable=missing-function-docstring, missing-module-docstring

a = [[1, 2, 3]]
a.append((4.4, 8.4))

Loading