Skip to content

empty Unions as empty lists #184

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.

Sign up for GitHub

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

Draft
campospinto wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

empty Unions as empty lists #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f686f7
empty Union as empty list
campospinto Dec 10, 2025
2d92ce0
try with empty tuple
campospinto Dec 10, 2025
4f22ba1
test emptyness as None or ()
campospinto Dec 10, 2025
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
4 changes: 2 additions & 2 deletions sympde/topology/basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@ def __new__(cls, *args):
# remove duplicates and sort domains by their string representation
args = sorted(set(args), key=str)

# a. If the required Union contains no domains, return None;
# a. If the required Union contains no domains, return an empty tuple;
# b. If it contains a single domain, return the domain itself;
# c. If it contains multiple domains, create a Union object.
if not args:
obj = None
obj = ()
elif len(args) == 1:
obj = args[0]
else:
Comment on lines -139 to 146
Copy link
Member

@yguclu yguclu Dec 11, 2025

Choose a reason for hiding this comment

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

I am not sure I understand why we need this change for the case of an empty Union, but not for a Union containing a single domain. If being able to iterate is really important, why not doing it for all cases?

Copy link
Collaborator Author

@campospinto campospinto Dec 11, 2025

Choose a reason for hiding this comment

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

Indeed, single domain unions should be handled consistently.
I just wanted to see whether the modification for empty unions was passing the tests before looking at the case of a single element.

I have no idea whether "unions" should be iterable objects, I just saw that we iterate on them in some places and currently this raises an error when the union is empty.

Copy link
Collaborator Author

@campospinto campospinto Dec 11, 2025

Choose a reason for hiding this comment

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

ps -- according to the mathematical usage, a "union" of domains (sets in Rd) is a priori a domain itself, so it's unclear what we should get when iterating over it: subdomains ? points ?

Maybe the issue comes from the fact that some objects expected to be iterable (like interiors, boundaries or interfaces ) are currently defined as "unions"...

Copy link
Collaborator Author

@campospinto campospinto Dec 12, 2025

Choose a reason for hiding this comment

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

For now I'm turning this into a draft -- we should probably discuss in issue #183 how to better solve this problem

Expand Down
14 changes: 7 additions & 7 deletions sympde/topology/domain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ def __new__(cls, name : str, *,
# ...

# ...
if ( ( interiors is None ) and ( connectivity is None ) and ( dim is None) ):
if ( ( interiors in [None, ()] ) and ( connectivity in [None, ()] ) and ( dim is None) ):
raise ValueError('> either interiors or connectivity must be given')
# ...

# ...
if not( interiors is None ):
if not( interiors in [None, ()] ):
if not isinstance( interiors, (*iterable_types, InteriorDomain)):
raise TypeError('> Expecting an iterable or a InteriorDomain')

Expand All @@ -103,7 +103,7 @@ def __new__(cls, name : str, *,
interiors = Tuple(*interiors)
# ...

if not( boundaries is None ):
if not( boundaries in [None, ()] ):
if not isinstance( boundaries, (*iterable_types, Boundary)):
raise TypeError('> Expecting an iterable or a Boundary')

Expand All @@ -119,7 +119,7 @@ def __new__(cls, name : str, *,

boundaries = Tuple(*boundaries)

if not( connectivity is None ):
if not( connectivity in [None, ()] ):
if not isinstance( connectivity, Connectivity ):
raise TypeError('> Expecting a Connectivity')

Expand All @@ -128,7 +128,7 @@ def __new__(cls, name : str, *,
connectivity = Connectivity()

# ...
if interiors is None and dim:
if interiors in [None, ()] and dim:
interiors = [InteriorDomain(name, dim=dim)]

if len(interiors) == 0 and dim is None:
Expand Down Expand Up @@ -228,7 +228,7 @@ def interfaces(self) -> TypeUnion[Union, Interface, None]:
@property
def corners(self):
corners = getattr(self,'_corners', None)
if corners is None:
if corners in [None, ()]:
corners = self.get_shared_corners()
self._corners = corners
return corners
Expand Down Expand Up @@ -540,7 +540,7 @@ def join(cls, patches, connectivity, name):

# ... boundary
boundaries = Union(*[b for p in patches for b in p.boundary]).complement(Union(*boundaries))
if boundaries is None:
if boundaries in [None, ()]:
boundaries = ()
else :
boundaries = boundaries.as_tuple()
Expand Down