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

Fix 86 #87

Merged
merged 3 commits into from Nov 8, 2020
Merged

Fix 86 #87

merged 3 commits into from Nov 8, 2020

Conversation

zakv
Copy link
Contributor

@zakv zakv commented Nov 7, 2020

This PR resolves #86 by making Run.get_units() only return units for the group specified by its optional group argument. If it is left at its default value of None, then the units for all globals in all groups are returned as before. Also, a docstring was added for that method.

@philipstarkey
Copy link
Member

philipstarkey commented Nov 7, 2020

Wondering if the following is a cleaner implementation (the append_units function seems overly complex for what it does).

    def get_units(self, group=None):
        units = {}
        path = 'globals'
        if group is not None:
            path += f'/{group}'
        with h5py.File(self.h5_path, 'r') as h5_file:
            h5_file[path].visititems(
                lambda n, o: units.update(dict(o.attrs) if 'units' in n else {})
            )
        return units

(This has not been tested. I'm not even sure if that dict above is necessary either)

There are two main changes in this:

  1. Dynamically constructing path rather than using if/else
  2. Ditching the function for a lambda.

The first change is probably cleaner, but second might not be worth it. Could instead use:

        def append_units(name, obj):
            if 'units' in name:
                units.update(dict(obj.attrs))

(I think)

EDIT: Oh, and these examples are missing the try/except to catch invalid group name!

@zakv
Copy link
Contributor Author

zakv commented Nov 7, 2020

Yeah you're right that method could be simplified a bit. I still went with append_units() over the lambda function just because I thought it made the code a bit easier to read/understand, though maybe a bit less pythonic.

@philipstarkey philipstarkey merged commit eaca856 into labscript-suite:master Nov 8, 2020
@zakv zakv deleted the fix-86 branch November 8, 2020 02:30
dihm added a commit that referenced this pull request Dec 7, 2021
commit 8575ff2
Author: David Meyer <dihm@users.noreply.github.com>
Date:   Tue Dec 7 10:52:15 2021 -0500

    Update setup.cfg to show python 3.9 support

commit e4b57f8
Merge: 8bf02a2 915cc1c
Author: David Meyer <dihm@users.noreply.github.com>
Date:   Tue Dec 7 07:31:45 2021 -0500

    Merge pull request #95 from dihm/msg_replacement

    Msg replacement

commit 8bf02a2
Merge: 72f82b3 a98a766
Author: David Meyer <dihm@users.noreply.github.com>
Date:   Tue Dec 7 07:30:06 2021 -0500

    Merge pull request #88 from zakv/move-infer_objects

    Move infer_objects() out of main thread

commit 915cc1c
Author: David Meyer <dihm@users.noreply.github.com>
Date:   Fri Dec 3 09:01:04 2021 -0500

    Adding the extra bugfixes found in #91.

commit fa9eedb
Author: David Meyer <dihm@users.noreply.github.com>
Date:   Fri Dec 3 09:00:29 2021 -0500

    Replace deprecated msgpack with python's pickle for saving dataframes.

    This keeps machinery in place to read msgpack files if the installed
    pandas supports it, but it will always save new dataframes as pickles.

commit 72f82b3
Merge: cb60667 26ef795
Author: David Meyer <dihm@users.noreply.github.com>
Date:   Fri Jul 16 16:09:52 2021 -0400

    Merge pull request #93 from dihm/update_lyse_docs

    Update lyse docs to match autogenerated docs from other modules.

commit 26ef795
Author: David Meyer <dihm.meyer@gmail.com>
Date:   Thu Jul 15 10:30:09 2021 -0400

    Add docstring coverage to build.

commit 98cac68
Author: David Meyer <dihm.meyer@gmail.com>
Date:   Thu Jul 15 10:29:57 2021 -0400

    Update sphinx pin.

commit 4671ef6
Author: David Meyer <dihm.meyer@gmail.com>
Date:   Tue Jul 13 17:21:29 2021 -0400

    Add docstrings to the API functions.

    Reasonably sure they are all accurate.

commit 40d207c
Author: David Meyer <dihm.meyer@gmail.com>
Date:   Mon Jul 12 10:47:31 2021 -0400

    Tweaked docs formatting.

commit 47b60a8
Author: David Meyer <dihm.meyer@gmail.com>
Date:   Mon Jul 12 10:40:42 2021 -0400

    Convert doc generation to recursive autosummary call.

commit 9add58b
Author: David Meyer <dihm.meyer@gmail.com>
Date:   Mon Jul 12 10:38:50 2021 -0400

    Tweak docstrings.

commit 3873dcc
Author: David Meyer <dihm.meyer@gmail.com>
Date:   Mon Jul 12 09:26:55 2021 -0400

    Add Qt5 inventory for intersphinx lookup.

commit e7cf19a
Author: David Meyer <dihm.meyer@gmail.com>
Date:   Mon Jul 12 09:26:31 2021 -0400

    Update sphinx pins to match rest of suite docs.

commit cb60667
Merge: eaca856 8596dea
Author: Chris Billington <chrisjbillington@gmail.com>
Date:   Fri Jan 29 14:00:36 2021 +1100

    Merge pull request #89 from chrisjbillington/master

    Do not use deprecated set-env command

commit 8596dea
Author: chrisjbillington <chrisjbillington@gmail.com>
Date:   Fri Jan 29 13:57:39 2021 +1100

    Do not use deprecated set-env command

commit a98a766
Author: Zak V <zakven@mit.edu>
Date:   Thu Nov 19 14:13:34 2020 -0500

    infer_objects() is now run outside of the main thread when data() is called to reduce main thread contention.

commit eaca856
Merge: fcfda48 bcede15
Author: Phil Starkey <philipstarkey@users.noreply.github.com>
Date:   Sun Nov 8 12:11:30 2020 +1100

    Merge pull request #87 from zakv/fix-86

    Fix 86

commit bcede15
Author: Zak V <zakven@mit.edu>
Date:   Sat Nov 7 17:33:51 2020 -0500

    Simplified flow control and append_units() funciton in Run.get_units().

commit e56d5fa
Author: Zak V <zakven@mit.edu>
Date:   Fri Nov 6 20:51:33 2020 -0500

    Added docstring for Run.get_units().

commit 6991aa9
Author: Zak V <zakven@mit.edu>
Date:   Fri Nov 6 20:50:38 2020 -0500

    Run.get_units() now uses its optional group argument

commit fcfda48
Merge: cdef626 1dac68f
Author: Phil Starkey <philipstarkey@users.noreply.github.com>
Date:   Thu Nov 5 17:02:14 2020 +1100

    Merge pull request #80 from zakv/fix-78

    Fix 78

commit 1dac68f
Author: Zak V <zakven@mit.edu>
Date:   Wed Nov 4 12:54:52 2020 -0500

    Run.__init__() now properly handles when a lyse script is called analysis_subprocess.py.

commit 7ba7062
Author: Zak V <zakven@mit.edu>
Date:   Wed Nov 4 03:51:30 2020 -0500

    Changed Run's private attributes from single underscore to double underscore.

commit 3d38b3b
Author: Zak V <zakven@mit.edu>
Date:   Wed Nov 4 03:45:40 2020 -0500

    Refactored Sequence.__init__() to use Run.__init__() per Phil's idea in PR #80.

commit cdef626
Merge: 695c220 86ec2a6
Author: Phil Starkey <philipstarkey@users.noreply.github.com>
Date:   Wed Nov 4 16:39:00 2020 +1100

    Merge pull request #79 from philipstarkey/philipstarkey/issue77

    Fixes #77

commit 695c220
Merge: 0910447 f80f182
Author: Phil Starkey <philipstarkey@users.noreply.github.com>
Date:   Wed Nov 4 16:35:54 2020 +1100

    Merge pull request #76 from zakv/dataframe-subset

    Dataframe subset

commit f80f182
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 20:21:38 2020 -0500

    Corrected formatting in data()'s docstring.

commit ab8e44f
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 20:07:41 2020 -0500

    Corrected indentation in Run.group's docstring.

commit 005163e
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 20:06:50 2020 -0500

    Fixed issues with Sequence.__init__().

    First, it was updated to work with recent changes to its parent class. Also it used to error out if the hdf5 file didn't already exist, as first pointed out in PR 73, but that is now resolved..

commit 6a2755b
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 16:44:09 2020 -0500

    Replaced double underscores with single for private Run attributes so that child classes, e.g. Sequence, can use them.

commit c6d8306
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 15:49:50 2020 -0500

    Corrected typos in lyse.save_result()'s docstring.

commit e7778b5
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 15:49:17 2020 -0500

    Updated docstring for Run.save_result_array().

commit 9a590e5
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 15:32:58 2020 -0500

    Removed some trailing whitespace in lyse.Run().

commit cad1435
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 15:32:31 2020 -0500

    Updated error messages in Run.save_result_array() and replaced Exception with PermissionError to be more specific.

commit 9af179c
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 15:22:28 2020 -0500

    Cleaned up error messages in Run.save_result() and changed Exception to PermissionError to be more specific.

commit 60f00a4
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 15:04:28 2020 -0500

    Replaced "run" with "shot" in Run's docstring.

commit 575fb4f
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 14:56:32 2020 -0500

    Fixed minor typo in comment in Run class.

commit 0bd4a02
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 14:54:46 2020 -0500

    Changed Run's h5_path, no_write, and group attributes into properties.

commit d6c6919
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 14:29:47 2020 -0500

    Small update to docstring for Run.save_result().

commit c4203f3
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 14:25:28 2020 -0500

    Removed print statement from Run.save_results().

commit 5281dea
Author: Zak V <zakven@mit.edu>
Date:   Tue Nov 3 14:24:43 2020 -0500

    Updated docstrings for Run.save_result() and Run.save_results().

commit 9132cf3
Author: Zak V <zakven@mit.edu>
Date:   Mon Nov 2 16:31:23 2020 -0500

    Run._create_group_if_not_exists() can now handle another thread/process creating the hdf5 group partway through its execution.

commit 997a6c7
Author: Zak V <zakven@mit.edu>
Date:   Mon Nov 2 15:50:52 2020 -0500

    Corrected some typos in strings in __init__.py.

commit 565c1b3
Author: Zak V <zakven@mit.edu>
Date:   Mon Nov 2 15:45:35 2020 -0500

    Added a docstring for the Run class.

commit 029fdda
Author: Zak V <zakven@mit.edu>
Date:   Mon Nov 2 15:08:02 2020 -0500

    Added docstring for Run.set_group().

commit afd9d8f
Author: Zak V <zakven@mit.edu>
Date:   Mon Nov 2 14:19:03 2020 -0500

    Simplified Run.group's behavior when initialized outside of a lyse script.

commit bff7d19
Author: Zak V <zakven@mit.edu>
Date:   Mon Nov 2 10:41:33 2020 -0500

    Fixed some formatting in data()'s docstring.

commit 86ec2a6
Author: Phil Starkey <philipstarkey@users.noreply.github.com>
Date:   Mon Nov 2 17:22:49 2020 +1100

    Fixed missing import

commit 9eff3cd
Author: Phil Starkey <philipstarkey@users.noreply.github.com>
Date:   Mon Nov 2 17:20:07 2020 +1100

    Fixes #77

    We now mock the missing methods in the site module.

commit d79bada
Author: Zak V <zakven@mit.edu>
Date:   Sat Oct 31 16:47:01 2020 -0400

    Reduced the amount of code run with @inmain_decorator() in WebServer.handler().

commit 8d2a69e
Author: Zak V <zakven@mit.edu>
Date:   Fri Oct 30 20:35:03 2020 -0400

    Added intersphinx link for Dataframe.filter() in data()'s docstring.

commit 4c944dd
Author: Zak V <zakven@mit.edu>
Date:   Fri Oct 30 12:43:37 2020 -0400

    Updated the error message raised in data() when the server is running an outdated version of lyse.

commit f26aba2
Author: Zak V <zakven@mit.edu>
Date:   Fri Oct 30 12:32:55 2020 -0400

    Moved df.sort_index() from data() to _rangeindex_to_multiindex().

commit 0910447
Merge: 72fa369 ef03a15
Author: Phil Starkey <philipstarkey@users.noreply.github.com>
Date:   Fri Oct 30 16:56:33 2020 +1100

    Merge pull request #74 from zakv/n_runs

    Add n_runs column to lyse dataframe

commit 72fa369
Merge: da9de67 8803447
Author: Phil Starkey <philipstarkey@users.noreply.github.com>
Date:   Fri Oct 30 16:53:22 2020 +1100

    Merge pull request #72 from zakv/set-group

    Make Run.set_group() create the group if necessary.

commit b344e9e
Author: Zak V <zakven@mit.edu>
Date:   Thu Oct 29 23:57:09 2020 -0400

    Simplified lyse.data() communication and made it backwards compatible.

commit 8c7a456
Author: Zak V <zakven@mit.edu>
Date:   Thu Oct 29 22:21:57 2020 -0400

    Fixed an issue where the conversion to multiindex in lyse.data() could fail when using its filter_kwargs argument.

commit b61ab25
Author: Zak V <zakven@mit.edu>
Date:   Thu Oct 29 19:30:50 2020 -0400

    Updated docstring for lyse.data().

commit f1be302
Author: Zak V <zakven@mit.edu>
Date:   Thu Oct 29 19:14:43 2020 -0400

    Added optional filter_kwargs argument to lyse.data().

commit d134b6e
Author: Zak V <zakven@mit.edu>
Date:   Thu Oct 29 12:03:23 2020 -0400

    Improved WebServer's thread safety.

commit d1d9897
Author: Zak V <zakven@mit.edu>
Date:   Wed Oct 28 21:42:03 2020 -0400

    Clarified lyse.data()'s docstring about behavior when n_sequences is greater than number of sequences available.

commit c25caef
Author: Zak V <zakven@mit.edu>
Date:   Wed Oct 28 20:36:02 2020 -0400

    Added a docstring for lyse.data().

commit 9d662ea
Author: Zak V <zakven@mit.edu>
Date:   Wed Oct 28 20:00:43 2020 -0400

    lyse.data() now correctly handles when n_sequences=0.

commit c75b945
Author: Zak V <zakven@mit.edu>
Date:   Wed Oct 28 19:01:36 2020 -0400

    Added optional n_sequences argument to lyse.data().

commit ef03a15
Author: Zak V <zakven@mit.edu>
Date:   Wed Oct 21 07:54:18 2020 -0400

    Added n_runs column to lyse dataframe.

commit 8803447
Author: Zak V <zakven@mit.edu>
Date:   Thu Jul 16 16:58:30 2020 -0400

    Run.set_group() now creates the group if necessary.
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

Successfully merging this pull request may close these issues.

Run.get_units() doesn't use its group argument
2 participants