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

Improve union method for a list of MOC #21

Closed
ggreco77 opened this issue Jun 15, 2018 · 8 comments
Closed

Improve union method for a list of MOC #21

ggreco77 opened this issue Jun 15, 2018 · 8 comments

Comments

@ggreco77
Copy link

ggreco77 commented Jun 15, 2018

It could be nice to improve the union method for a list of MOC - MOC.union(list).

@ggreco77
Copy link
Author

ggreco77 commented Jul 2, 2018

I tried to add a new method to the class AbstractMoc named union_moc_list for fast union of a list of moc

def union_moc_list(self, another_mocs = [] ):
    """
    Union between self and a list of mocs

    Parameters
    ----------
    another_mocs : `~mocpy.abstract_moc.AbstractMoc`
        a list of MOC/TimeMoc to bind to self

    Returns
    -------
    result : `~mocpy.moc.MOC` or `~mocpy.tmoc.TimeMoc`
        MOC object whose interval set corresponds to : self | ``moc``

    """
    for another_moc in another_mocs:
        iv_set_union = self._interval_set.union(another_moc._interval_set)
        result = self.__class__.from_interval_set(iv_set_union)
        self = result

    return result

@bmatthieu3
Copy link
Collaborator

@ggreco77 What do you think of redefining the current AbstractMOC.union method like that:
def union(self, *args)

You can call the method like that:
moc1.union(moc2, moc3, ..., mocN)
Or if you have a list of MOCs :
moc1.union(*moc_l)

@bmatthieu3
Copy link
Collaborator

Or even like that def union(self, another_moc, *args) so that at least one moc is mandatory for computing an union of MOCs.

@tboch
Copy link
Collaborator

tboch commented Jul 6, 2018

@ggreco77 What do you think of redefining the current AbstractMOC.union method like that:
def union(self, *args)

You can call the method like that:
moc1.union(moc2, moc3, ..., mocN)
Or if you have a list of MOCs :
moc1.union(*moc_l)

That would be my preferred API, def union(self, another_moc, *args) looks less natural to me.

@ggreco77
Copy link
Author

ggreco77 commented Jul 6, 2018

Let me show you an real application of such new method
http://nbviewer.jupyter.org/gist/ggreco77/c114dd9f7364a216bd83602cee156ca3
as reported in "Ex. 1 Group the Observation in g filter - measure the sky area and the skymap intersection"
in the first lines

many thanks

@ggreco77
Copy link
Author

I got some troubles with the union method implemented as union(self, another_moc, *args);

mocs= [moc1, moc2, moc3, moc4]

for moc in mocs: uni_mocs = MOC.union(moc)

How can obtain such statement using for or similar passing a list?

thanks!

@bmatthieu3
Copy link
Collaborator

Union/difference/intersection are not static (hmm just thinking it may be a good idea to do static functions taking multiple mocs through *args).

But for now if you have one moc object and a list of other mocs other_mocs_l=[moc2, ..., mocN] you can do the union of moc with the list by doing:
uni_mocs = moc.union(*other_mocs_l) or uni_mocs = moc.union(moc2, moc3, ..., mocN)
The * operator transforms a list say l=[1,2,3] in the following of values 1, 2, 3 that can be given in a function through the *args parameter name.
uni_mocs stores the result. moc is not changed after the method call. No needs to do a for loop.

@ggreco77
Copy link
Author

Very clear, the method perfectly works in my scripts.
Many thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants