Db monitor #52

hover2pi · 2018-04-02T16:58:27Z

Wrote database monitor to count files in MAST #48

bourque · 2018-04-11T17:52:57Z

Sorry for the delay @hover2pi, I'll look at this tomorrow or Friday

bourque

Thanks @hover2pi, very nicely written. I have a few questions and requests, but the code is shaping up nicely.

bourque · 2018-04-02T21:28:37Z

jwql/dbmonitor/dbmonitor.py

+    >>> inventory, keywords = dbmonitor.jwst_inventory()
+
+"""
+import sys


@hover2pi Please order imports based on the PEP 8 suggestion of three groups (and in alphabetical order)

Imports should be grouped in the following order: 1. standard library imports 2. related third party imports 3. local application/library specific imports You should put a blank line between each group of imports.

bourque · 2018-04-02T21:31:32Z

jwql/dbmonitor/dbmonitor.py

+from astroquery.mast import Mast
+import astropy.table as at
+
+JWST_INSTRUMENTS = ['NIRISS', 'NIRCam', 'NIRSpec', 'MIRI', 'FGS']


@hover2pi It would probably be a good idea to stick global lists like these in the utils.py module and import them. Then they could easily be used by other modules. Could you put those in there?

bourque · 2018-04-03T21:37:08Z

jwql/dbmonitor/dbmonitor.py

+                     'EVENTLIST', 'CUBE', 'CATALOG', 'ENGINEERING', 'NULL']
+
+
+def listMissions():


@hover2pi Per our style guide, could you order the function definitions in alphabetical order?

bourque · 2018-04-13T15:25:45Z

jwql/dbmonitor/dbmonitor.py

+    return_data: bool
+        Return the actual data instead of counts only
+
+    Returns


@hover2pi You might want to add a note here that all results are returned if return_data is True

bourque · 2018-04-13T15:28:58Z

jwql/dbmonitor/dbmonitor.py

+               {'paramName': 'dataproduct_type', 'values': dataproduct}]
+
+    # Include additonal filters
+    for k, v in add_filters.items():


@hover2pi PEP 8 discourages single letter variable names, but this naming convention seems like somewhat of a standard amongst the python community, so I'll allow it! I'm only mentioning this because I have given @bhilbert4 grief about this before and I want to play fair ;)

bourque · 2018-04-13T15:40:28Z

jwql/dbmonitor/dbmonitor.py

+    return request
+
+
+def mastQuery(request):


@hover2pi Could you instead use the MAST API directly, instead of having to establish connections and sending POSTs? i.e.:

response = astroquery.mast.Mast.service_request_async('Mast.Caom.Filtered', params) result = resonse[0].json()

Similar to how we are querying MAST in this notebook. It seems simpler to me.

bourque · 2018-04-13T15:43:46Z

jwql/dbmonitor/dbmonitor.py

+    count = instrument_inventory(instruments, dataproduct=dataproducts)
+    inventory.add_row(['*', '*', count])
+
+    # Retrieve one dataset to get header keywords


@hover2pi It will be nice to have an understanding of these keywords that come out of the Mast.Caom.Filtered service, but I would also like to be tracking the number of keywords that come out of the instrument specific services (e.g. Mast.Jwst.Filtered.Niriss). Could that be added?

…modapi generated docs for now. #51

first pass at getting sphinx and automodapi running

Filename parser utility function

All changes were made

hover2pi · 2018-05-02T16:14:26Z

Ok @bourque , this should be ready to go. I'm going to add the plots in the Web app in a separate branch.

…_monitor

hover2pi · 2018-05-11T19:59:23Z

@bourque I fixed some docstrings, modified the queries to accept filters (just in case), and broke out the keyword queries into their own functions so feeding it to the Web app is easier.

bourque · 2018-05-14T14:44:47Z

Thanks @hover2pi, I'm going to try to review this today or tomorrow.

bourque

@hover2pi This looks and works great, and exactly what I had in mind.

My only comment is that it seems the jwst_inventory and jwst_inventory_caom functions are mostly the same and do not follow the DRY principle (Don't Repeat Yourself) very well. I wonder if it would be better to add a parameter called service and use conditions to distingish the calls to the instrument_inventory functions, i.e.

def jwst_inventory(service, instruments=JWST_INSTRUMENTS, dataproducts=['image', 'spectrum', 'cube'],
plot=False):

...

if service == 'filtered':
    count = instrument_inventory(instrument, dataproduct=dp)
elif service == 'caom':
    count = instrument_inventory_caom(instrument, dataproduct=dp)

...

if service == 'filtered':
    keywords[instrument] = instrument_keywords(instrument)
elif service == 'caom':
    keywords[instrument] = instrument_keywords(instrument)

...

What do you think?

bourque · 2018-05-14T15:51:43Z

jwql/utils/utils.py


+JWST_INSTRUMENTS = ['NIRISS', 'NIRCam', 'NIRSpec', 'MIRI', 'FGS']


@hover2pi Oops, somehow this got defined twice. Could you remove this?

hover2pi · 2018-05-23T16:29:10Z

@bourque I combined the CAOM and filtered queries and removed that weird duplicate global variable definition. Should be good to go.

bourque

Im not actually approving this now, just trying something out

bourque · 2018-05-30T19:15:41Z

@hover2pi Random question that may or may not have to do with our rebase vs pull discussion: which text editor do you use to develop?

…ing it. Using bokeh.charts instead of bkcharts.

…put plots.

bourque · 2018-05-31T15:46:42Z

@hover2pi This is good to go! Great work. I made a few additional commits to this, namely to save the plots to the jwql central storage area instead of just showing them, and some minor API doc formatting stuff.

Also, I had to switch to using bokeh.charts with bokeh v0.12.5 instead of bkcharts v0.2 in order to get this to work and the tests to pass. I wasn't able to get this to work with the latest version of bkcharts and the latest version of bokeh -- which versions were you using?

Anyways, this is a good working version, so I am going to merge it. I can already think of a few ways to improve these charts, but we can deal with that in a separate PR.

Db monitor

hover2pi added 6 commits April 2, 2018 11:01

Added functions to query MAST for record counts and data.

72a877d

Generates table of result counts

b52a0c5

PEP8 compliance.

9a363a5

Added total db counts

39a945d

Added total db counts comment

7ad6e74

Added keyword count to jwst_inventory()

61f16f9

hover2pi requested a review from bourque April 2, 2018 16:59

bourque previously requested changes Apr 13, 2018

View reviewed changes

bourque assigned hover2pi Apr 13, 2018

bourque added enhancement Database Package Medium Priority labels Apr 13, 2018

bourque and others added 16 commits April 16, 2018 10:36

Added some more acknowledgements

c4324a1

Add Joe Hunkeler's filename parser to utils.py with minor modifications

ef7c148

Update references/authors

f55f25f

first pass at getting sphinx and automodapi running, not storing auto…

86cd388

…modapi generated docs for now. #51

Adding readthedocs files, issue #51

b700d07

deleting these rtd files for now, rtd will not run on a private repo

cf5c37a

Updated environment file and a small conf tweak.

aa88a23

Merge pull request #53 from SaraOgaz/sphinx_setup

d95d028

first pass at getting sphinx and automodapi running

Create simply pytests for utils.get_config and utils.filename_parser

b31cdf7

Updating docstrings, etc.

f865a2e

Removed some extra lines

72513a3

Minor formatting changes

4130d96

Merge pull request #63 from laurenmarietta/filename_parser

f69948c

Filename parser utility function

Moved global lists to utils.py and removed POST calls.

8f914ab

Added CAOM functions and non-CAOM functions.

087fde4

Added donut plot.

72d8e7b

hover2pi added 2 commits May 2, 2018 12:08

PEP8 compliance.

4b6f6e8

Merge conflicts.

3194f34

bourque and others added 3 commits May 4, 2018 14:43

Updated the module docstring to render more nicely in the API docs

b00f7c5

Fixed dbmonitor to accept filters and broke out keyword queries.

92761dc

Merge branch 'db_monitor' of https://github.com/hover2pi/jwql into db…

81c4e59

…_monitor

bourque reviewed May 14, 2018

View reviewed changes

hover2pi added 2 commits May 23, 2018 11:48

Combined CAOM and filtered service queries.

c899d1e

Removed duplicate global variables

59ef757

hover2pi added 5 commits May 23, 2018 14:12

PEP 8

b247a38

Started adding tests.

e0e735e

Formatting docstring.

6ce1f2d

Added a few unittests.

bcfe728

Fixed my name in the README

1b87971

bourque approved these changes May 30, 2018

View reviewed changes

bourque added 4 commits May 31, 2018 11:14

Saving the plot to the jwql central storage area instead of just show…

8ce3800

…ing it. Using bokeh.charts instead of bkcharts.

Added call to set_permissions for setting the permissions for the out…

c109a57

…put plots.

Added bokeh to list of requires.

d2f6337

Added astroquery dependency

0cf6175

bourque merged commit 88beea3 into spacetelescope:master May 31, 2018

bourque deleted the db_monitor branch May 31, 2018 15:47

This was referenced May 31, 2018

Add sphinx docs for db_monitor #82

Closed

Build database monitor #48

Closed

bourque added a commit to bourque/jwql that referenced this pull request Apr 2, 2019

Merge pull request spacetelescope#52 from hover2pi/db_monitor

30dade3

Db monitor

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Db monitor #52

Db monitor #52

hover2pi commented Apr 2, 2018 •

edited

Loading

bourque commented Apr 11, 2018

bourque left a comment

bourque Apr 2, 2018

bourque Apr 2, 2018

bourque Apr 3, 2018

bourque Apr 13, 2018

bourque Apr 13, 2018

bourque Apr 13, 2018

bourque Apr 13, 2018

hover2pi commented May 2, 2018 •

edited

Loading

hover2pi commented May 11, 2018

bourque commented May 14, 2018

bourque left a comment •

edited

Loading

bourque May 14, 2018

hover2pi commented May 23, 2018

bourque left a comment

bourque commented May 30, 2018

bourque commented May 31, 2018

		'EVENTLIST', 'CUBE', 'CATALOG', 'ENGINEERING', 'NULL']


		def listMissions():


		JWST_INSTRUMENTS = ['NIRISS', 'NIRCam', 'NIRSpec', 'MIRI', 'FGS']

Db monitor #52

Db monitor #52

Conversation

hover2pi commented Apr 2, 2018 • edited Loading

bourque commented Apr 11, 2018

bourque 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hover2pi commented May 2, 2018 • edited Loading

hover2pi commented May 11, 2018

bourque commented May 14, 2018

bourque left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hover2pi commented May 23, 2018

bourque left a comment

Choose a reason for hiding this comment

bourque commented May 30, 2018

bourque commented May 31, 2018

hover2pi commented Apr 2, 2018 •

edited

Loading

hover2pi commented May 2, 2018 •

edited

Loading

bourque left a comment •

edited

Loading