WIP: Regolith Grant Report Builder by vivianlin2000 · Pull Request #523 · regro/regolith

vivianlin2000 · 2020-07-05T18:01:33Z

No description provided.

codecov · 2020-07-05T18:06:14Z

Codecov Report

Merging #523 into master will decrease coverage by 0.00%.
The diff coverage is 58.33%.

@@            Coverage Diff             @@
##           master     #523      +/-   ##
==========================================
- Coverage   65.57%   65.57%   -0.01%     
==========================================
  Files          59       60       +1     
  Lines        5531     5560      +29     
==========================================
+ Hits         3627     3646      +19     
- Misses       1904     1914      +10

Impacted Files	Coverage Δ
regolith/builders/grantreportbuilder.py	`56.52% <56.52%> (ø)`
regolith/builder.py	`100.00% <100.00%> (ø)`
regolith/dates.py	`88.19% <0.00%> (+0.37%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16c3c25...6546f6e. Read the comment docs.

sbillinge

good to see this started! Already one request. I realize I made a mistake.....I don't have to make reports on my proposals, but on my grants, so it should be GrantReportBuilder, etc.

The normal way to do builders is to start with the report that is being build and to work back. If you want to have a quick meeting, that would work great. The closest builder may be the cv or the appraisal builder.

sbillinge

Hi Vivian, this looks great.

Pls see my comments.

Also another comment but I can't leave it inline for some reason, but load the gtx collections by looping over the needed_dbs list.

I will have more time starting next week, and maybe we can have a meeting and go over the different categories and where we are going to get the information from.....

sbillinge · 2020-07-18T01:40:45Z

regolith/builders/grantreportbuilder.py

        valid_presentations = []
-        for p in self.gtx["people"]:
-            if p["active"] and p["_id"] is not "sbillinge":
+        for p in people:


for person in people

sbillinge · 2020-07-18T01:41:59Z

regolith/builders/grantreportbuilder.py

+
+        # Get All Active Members
+        people = []
+        for p in self.gtx["people"]:


current_members = [person for person in self.gtx['people'] if person['active']]

sbillinge · 2020-07-18T01:43:48Z

regolith/builders/grantreportbuilder.py

-        for p in self.gtx["people"]:
-            if p["active"] and p["_id"] is not "sbillinge":
+        for p in people:
+            if p["_id"] is not "sbillinge":


my name shouldn't be hard-coded in here

sbillinge · 2020-07-18T01:44:57Z

regolith/builders/grantreportbuilder.py


+        # Individuals that have worked on project
+        individuals_data = []
+        for p in people:


you keep looping over people so why not put each of these actions inside a single outer loop over people?

sbillinge

👍

sbillinge · 2020-07-20T17:16:44Z

regolith/builders/grantreportbuilder.py

-            all_docs_from_collection(rc.client, "institutions"), key=_id_key
-        )
-
+        for dbs in rc.needed_dbs:


so pretty! 👍

sbillinge · 2020-07-20T17:17:20Z

regolith/builders/grantreportbuilder.py

-        for p in self.gtx["people"]:
-            if p["active"]:
-                people.append(p)
+        current_members = [person for person in self.gtx['people'] if person['active']]


sbillinge

nice progress. Pls see comment....

sbillinge · 2020-07-29T13:28:42Z

regolith/builders/grantreportbuilder.py

-        begin_year, begin_month, begin_day = int(today[0]) - 1, int(today[1]), int(today[2])
-        end_date = date(end_year, end_month, end_day)
-        begin_date = date(begin_year, begin_month, begin_day)
+        start_date = rc.start_date.split("-")


this is heading us back down a road to the old days (passing dates around as strings and ints) rather than the brave new world of working with dates. I very much doubt this is the direction we want to go in here....

Yea I realized this too! Will work with date objects instead of the strings and ints.

…porting period

sbillinge

pls see comments

sbillinge · 2020-07-29T19:12:59Z

regolith/builders/grantreportbuilder.py

-        end_year, end_month, end_day = int(end_date[0]), int(end_date[1]), int(end_date[2])
-        end_date = datetime.datetime(end_year, end_month, end_day)
-        begin_date = datetime.datetime(begin_year, begin_month, begin_day)
+        start_date = datetime.strptime(rc.start_date, "%Y-%m-%d")


we usually use a date parser from dateutils. pls could you look in one of the other helpers for the syntax?

sbillinge · 2020-07-29T19:15:20Z

regolith/builders/grantreportbuilder.py

+        prums = [prum for prum in self.gtx['projecta']
+                 if grant_name in prum['grants']
+                 and
+                 ((start_date <= datetime.strptime(prum['end_date'], "%Y-%m-%d") <= end_date


use get_dates to get dates and is_current etc. (fully tested functions in tools.py to do this work if poss.

sbillinge

nice....coming together! Very exciting!

sbillinge · 2020-07-30T10:57:02Z

regolith/templates/grantreport.txt

@@ -1,9 +1,8 @@
-NSF {{beginYear}}-{{endYear}} Annual Report by {{authorFullName}}, ({{ institution }})
+NSF {{beginYear}}-{{endYear}} Annual Report by Simon J. L. Billinge, (Columbia University)


I shouldn't be hard-coded in here.

sbillinge · 2020-07-30T11:00:28Z

regolith/templates/grantreport.txt

-Specific Objectives:
-Significant Results:
+------------------------------------------------------------------------------
+- Major Activities (currently worked on projecta):


again, can't have me hard-coded in here.

If there is something that is "me" specific that doesn't change often it should probably be in people. However, since this is a grant-specific thing it might better be in grants? If it is mostly me-specific but will be described in the grant report of multiple grants, this should probably be in people but have a field that is grant. This is how we discover how the world is organized!

sbillinge · 2020-07-30T11:01:19Z

regolith/templates/grantreport.txt

 {% endfor -%}}

 * How have the results been disseminated to communities of interest?
+1. Publications


This is an interesting one. It should probably be in people too so any user of Regolith can decide for themselves.

sbillinge

Nice progress! I left a few comments.

What is the progress. I wonder if I should merge this sometime soon and try and use it and give usage feedback?

sbillinge · 2020-08-04T15:07:44Z

regolith/builders/grantreportbuilder.py

-        # How have results been disseminated
+            for education in person['education']:
+                if 'phd' in education['degree'].lower() and 'columbia' in education['institution'].lower() and \
+                        rp_start_date.year <= education['end_year'] <= rp_end_date.year:


since you will be doing something like this a lot I guess, you may want to use a function. Can you use is_current which has already been written? I would probably try and use get_dates to get dates and then is_current.

It may not work, but using functions that are well tested and can handle edge cases well will save time later.

sbillinge · 2020-08-04T15:08:41Z

regolith/builders/grantreportbuilder.py

+        # Participants/Organizations
+        participants = {}
+        for person in grant_people:
+            p = self.gtx["people"].get(person)


please don't be lazy with variable names, it makes the code hard to read. What is p?

Here I think the logic is that p contains the person document for the person of name person, so I would pick variable names like this:

for name in grant_people: person = self.gtx["people"].get(person)

I think this logic assumes that the names in grant_people are valid id's is that right? This might be a bit brittle.

sbillinge · 2020-08-04T15:09:43Z

regolith/builders/grantreportbuilder.py

+            p = self.gtx["people"].get(person)
+            months = 0
+            if p['active']:
+                difference = datetime.today() - rp_start_date


is it correct to do this calculation from today?

sbillinge · 2020-08-04T15:10:23Z

regolith/templates/grantreport.txt

@@ -1,4 +1,4 @@
-NSF {{beginYear}}-{{endYear}} Annual Report by Simon J. L. Billinge, (Columbia University)
+NSF {{beginYear}}-{{endYear}} Annual Report


Don't you want a name here (just not my name hard-coded)

sbillinge · 2020-08-04T15:11:34Z

tests/outputs/grantreport/billinge_grant_report.txt

 3. Presentations at conferences and seminars

 * What do you plan to do during the next reporting period to accomplish the goals?
+#############################################################


this looks like copy-pasted text....maybe assign this string to a variable and use that?

sbillinge

good progress. Should we have a quick chat about this one of these days?

sbillinge · 2020-08-20T15:29:24Z

regolith/builders/grantreportbuilder.py



 def subparser(subpi):
+    subpi.add_argument("authorFullName", help="full name of the author of this grant report")


this is very brittle. I doubt I will ever remember the full name of people and whether it is capitalized and spaces and whatnot. We have two choices here.

require the person id (e.g., you would be vlin). This is easier to remember because we use a mostly standard naming scheme.

allow anything that is in id, name or alias and then use fuzzy retrieval to get the person back.

(2) is probably better because (1) is also covered by (2)

sbillinge · 2020-08-20T15:36:54Z

regolith/builders/grantreportbuilder.py

-                months = difference.year * 12 + difference.month
-            else:
+        for name in grant_people:
+            person = self.gtx["people"].get(name)


to get things from a collection we use things like fuzzy_retrieval in tools.py or find_one etc. in fsclient and mongoclient You might also find get_person from tools.py to be useful.

Create proposalreportbuilder.py

85feb2b

Create initial files

78d735d

sbillinge reviewed Jul 6, 2020

View reviewed changes

vivianlin2000 added 4 commits July 6, 2020 13:21

Changed Proposal Report Builder to Grant Report Builder

b9700cc

Added template

d549f98

progress

65e7261

more edits

83e178c

sbillinge reviewed Jul 18, 2020

View reviewed changes

vivianlin2000 changed the title ~~WIP: Regolith Proposal Report Builder~~ WIP: Regolith Grant Report Builder Jul 19, 2020

Suggestions Implemented

8e20314

sbillinge reviewed Jul 20, 2020

View reviewed changes

updates

db0168d

sbillinge reviewed Jul 29, 2020

View reviewed changes

Retrieved prums and people associated with grant and active during re…

d2410ba

…porting period

sbillinge reviewed Jul 29, 2020

View reviewed changes

More Updates

bc72627

sbillinge reviewed Jul 30, 2020

View reviewed changes

updates

0490554

sbillinge reviewed Aug 4, 2020

View reviewed changes

Updates

6546f6e

sbillinge reviewed Aug 20, 2020

View reviewed changes

		@@ -1,9 +1,8 @@
		NSF {{beginYear}}-{{endYear}} Annual Report by {{authorFullName}}, ({{ institution }})
		NSF {{beginYear}}-{{endYear}} Annual Report by Simon J. L. Billinge, (Columbia University)

		@@ -1,4 +1,4 @@
		NSF {{beginYear}}-{{endYear}} Annual Report by Simon J. L. Billinge, (Columbia University)
		NSF {{beginYear}}-{{endYear}} Annual Report



		def subparser(subpi):
		subpi.add_argument("authorFullName", help="full name of the author of this grant report")

Conversation

vivianlin2000 commented Jul 5, 2020

Uh oh!

codecov bot commented Jul 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Jul 5, 2020 •

edited

Loading