copy data from frontend to frontend #351

JoranAngevaare · 2020-11-17T08:29:36Z

What is the problem / what does the code in this PR do
Some frontends are harder to interface with than others. Using this new function, one is able to copy data from one frontend to another. This is especially useful in at least these two concrete examples:

Querying the database is in Mongo.py is expensive. Better save it to disk as that is quicker and prevents people from overloading the database.
The mongo backend is a nightmare to use outside of dali, it's much easier to transfer when backcasted to a normal DataDirectory frontend.

Can you briefly describe how it works?
Strax magic! I wrote it such that it can be used either trying to write to all the other frontends or to a specific one (

Can you give a minimal working example (or illustrate with a figure)?

for sf in st.storage[1:]:
    print(sf)
    print(st._is_stored_in_sf(run_id, target, sf))
st._copy_to_frontend(run_id, target, target_frontend_id=3)
for sf in st.storage[1:]:
    print(sf)
    print(st._is_stored_in_sf(run_id, target, sf))
## output
strax.storage.files.DataDirectory, path: /data/real_data/
True
strax.storage.files.DataDirectory, path: /data/_tests8/
False
strax.storage.files.DataDirectory, path: /data/_tests9/
False
# After copy
strax.storage.files.DataDirectory, path: /data/real_data/
True
strax.storage.files.DataDirectory, path: /data/_tests8/
False
strax.storage.files.DataDirectory, path: /data/_tests9/
True

Future requests
One might think that we may just as well then erase the original copy. This is is however not per se trivial in strax (yet), I might think about if we need this at some point.

JoranAngevaare · 2020-11-17T08:54:19Z

strax/context.py

@@ -1361,6 +1358,106 @@ def _apply_function(self, data, targets):
            data = function(data, targets)
        return data

+    def _copy_to_frontend(self, run_id, target,


Perhaps this should not be _-ed. If the reviewer has an opinion I'm interested

On the one hand I agree with you that this function should just be used by a very limited amount of people who know what they are doing and hence make the function not "visible". On the other hand I think it is very unlikely that anyone will use this function except for the experts. My/our concerns are very limited, and the underscore usually indicates that the function is a private member, I agree with your statement to remove it.

WenzDaniel

Look good. Thanks a lot.

WenzDaniel · 2020-11-20T07:12:15Z

strax/context.py

+            raise strax.DataNotAvailable(f'Cannot copy {run_id} {target} since it '
+                                         f'does not exist')
+        if len(strax.to_str_tuple(target)) > 1:
+            raise ValueError('_copy_to_frontend only works for ')


The message is missing something

whoop, thanks will add

WenzDaniel · 2020-11-20T07:34:12Z

strax/context.py

+            target_sf = self.storage
+        elif len(self.storage) > target_frontend_id:
+            # only write to selected other frontend
+            target_sf = [self.storage[target_frontend_id]]


I am wondering if it would not be a bit neater to add a fornt_end_id to the properties of every forntned. In that case one could precisely distinguish between frontends. However, this would also require some additional checks, e.g. that the same id is not specified twice.

What would be the gain? Now it's just the index in st.storage which is a list. Using a number is quite simple.

Could try to match the __repr__ or the input but that sounds more convoluted. Could do though because you than don't by accident fill the wrong sf.

I agree, I think it is not really needed and I am fine with the PR as it is.

Could do though because you than don't by accident fill the wrong sf.

This was also my concern, but I am not sure if the "risk" justifies the extra work.

actually, I'm not that sure if it makes sense. the repr is quite a long string and just referring to the class name will also not be sufficient.

There is not that much real risk as we are in the end checking if the sf is susceptible at all anyways

WenzDaniel · 2020-11-20T07:42:36Z

strax/context.py

@@ -1361,6 +1358,106 @@ def _apply_function(self, data, targets):
            data = function(data, targets)
        return data

+    def _copy_to_frontend(self, run_id, target,


On the one hand I agree with you that this function should just be used by a very limited amount of people who know what they are doing and hence make the function not "visible". On the other hand I think it is very unlikely that anyone will use this function except for the experts. My/our concerns are very limited, and the underscore usually indicates that the function is a private member, I agree with your statement to remove it.

darrylmasson

Daniel beat me to a few comments

darrylmasson · 2020-11-17T09:37:47Z

strax/context.py

        for sf in self.storage:
-            try:
-                sf.find(key, **self._find_options)
+            if self._is_stored_in_sf(run_id, target, sf):


A possible more compact alternate could be return any(map(partial(self._is_stored_in_sf, run_id, target), self.storage)), but this boolean-optimizes (as it were).

I thought about something like this but I decided against it. I wrote it like this as it returns as soon as a single one is true, you get the True flag. If one writes a SF that takes long to see if something is stored, you may safe some time.

darrylmasson · 2020-11-17T10:03:19Z

strax/context.py

+        if source_sf is None and raise_error:
+            raise ValueError('This cannot happen, we just checked that this '
+                             'run should be stored?!?')
+


Missing return?

oh my... thanks

darrylmasson · 2020-11-17T10:06:19Z

strax/context.py

+        # Keep frontends that:
+        #  1. already have the data; and
+        #  2. take the data; and
+        #  3. are readonly


JoranAngevaare added 2 commits November 17, 2020 09:04

copy data from frontend to frontend

ab4231d

Check frontend_id vs storage length

5f58da6

JoranAngevaare requested a review from darrylmasson November 17, 2020 08:31

JoranAngevaare added 3 commits November 17, 2020 09:36

fix codefactor

d138ca4

fix codefactor

c56262f

add docstrings

5adf425

JoranAngevaare commented Nov 17, 2020

View reviewed changes

JoranAngevaare requested a review from WenzDaniel November 19, 2020 19:38

WenzDaniel approved these changes Nov 20, 2020

View reviewed changes

darrylmasson approved these changes Nov 20, 2020

View reviewed changes

JoranAngevaare added 2 commits November 20, 2020 13:55

fix copy_to_frontend after review

d0b75d4

Merge branch 'master' into copy_to_sf

2ad58fd

JoranAngevaare merged commit 9bbe2e1 into master Nov 20, 2020

JoranAngevaare deleted the copy_to_sf branch November 20, 2020 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

copy data from frontend to frontend #351

copy data from frontend to frontend #351

JoranAngevaare commented Nov 17, 2020 •

edited

JoranAngevaare Nov 17, 2020

WenzDaniel Nov 20, 2020

WenzDaniel left a comment

WenzDaniel Nov 20, 2020

JoranAngevaare Nov 20, 2020

WenzDaniel Nov 20, 2020

JoranAngevaare Nov 20, 2020 •

edited

WenzDaniel Nov 20, 2020 •

edited

JoranAngevaare Nov 20, 2020

WenzDaniel Nov 20, 2020

darrylmasson left a comment

darrylmasson Nov 17, 2020

JoranAngevaare Nov 20, 2020

darrylmasson Nov 17, 2020

JoranAngevaare Nov 20, 2020

darrylmasson Nov 17, 2020

copy data from frontend to frontend #351

copy data from frontend to frontend #351

Conversation

JoranAngevaare commented Nov 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WenzDaniel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoranAngevaare Nov 20, 2020 • edited

Choose a reason for hiding this comment

WenzDaniel Nov 20, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darrylmasson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoranAngevaare commented Nov 17, 2020 •

edited

JoranAngevaare Nov 20, 2020 •

edited

WenzDaniel Nov 20, 2020 •

edited