Fix #2394: Add docstring to core.domain.user_jobs_continous.py #3569

shubha1593 · 2017-06-24T06:05:30Z

Fix #2394 : Add docstring for the functions in proper format.

seanlip

Thanks, @shubha1593 -- this is a great start! Please see inline comments.

seanlip · 2017-06-24T22:53:34Z

core/domain/user_jobs_continuous.py

+            user_id: str. The unique ID of the user
+
+        Returns:
+            tuple(job_queued_msec, output):


tuple(float, list(dict)). A 2-tuple with the following entries: - float. The time, in milliseconds since the Epoch, when the job was queued. - list(dict). A list of ... with keys: - 'type': str. Either ... - 'activity_id': str. The ID of the ... - ...

Note, for the second arg, how the types of each key of the dict are specified. The descriptions for the other fields need to be completed as well.

seanlip · 2017-06-24T22:54:28Z

core/domain/user_jobs_continuous.py

                type, such as feconf.UPDATE_TYPE_EXPLORATION_COMMIT.
-            delete_commit_message: This (string) represents the commit message
+            delete_commit_message: str. This represents the commit message
                to use when an activity is found to be deleted, such as
                feconf.COMMIT_MESSAGE_EXPLORATION_DELETED.

        Returns:
            A tuple with two entries:


See above for notes on format:

tuple(...). A tuple with two entries: - ...

seanlip · 2017-06-24T22:56:04Z

core/domain/user_jobs_continuous.py

+        Computes most recent activity commits and feedbacks of a spcecific user.
+
+        Args:
+            item: The parameter passed to this function is a single element of


Can we just say:

item: UserSubscriptionsModel. An instance of UserSubscriptionsModel.

Seems more concrete.

seanlip · 2017-06-24T22:57:12Z

core/domain/user_jobs_continuous.py

+
+        Returns:
+            This function may yield as many times as appropriate (including zero) 
+            the key/value 2-tuples. 


"the key/value 2-tuples. For example" --> "2-tuples in the format"

and combine into a single sentence.

seanlip · 2017-06-24T22:57:24Z

core/domain/user_jobs_continuous.py

+            For example, (reducer_key, recent_activity_commit_dict) where
+                reducer_key: str. This is of the form 'user_id@job_queued_msec'
+                recent_activity_commit_dict: dictionary with the following keys
+                    - 'type': The value of the commit_type argument.


Please add type info (see above).

seanlip · 2017-06-24T23:02:47Z

core/domain/user_jobs_continuous.py

+        Returns:
+            This function may yield as many times as appropriate the key/value 2-tuples. 
+            For example, (id, exploration_data) where
+                id: str. The unique id of contributor or owner of the exploration


Reframe this so that it's more centered on the user, rather than on the exploration. E.g.:

id: str. A user ID.
exploration_data: dict. A dict that includes entries for all the explorations that this user contributes to or owns. Each entry has the following keys: ...

seanlip · 2017-06-24T23:03:00Z

core/domain/user_jobs_continuous.py

+                    - 'num_ratings_for_owned_exp': int. Total number of ratings for the
+                            exploration
+        """
+


Remove newline.

seanlip · 2017-06-24T23:03:07Z

core/domain/user_jobs_continuous.py

+            stringified_values: A list of information regarding all the explorations
+                owned by user.
+        """
+


Remove newline.

seanlip · 2017-06-24T23:03:15Z

core/domain/user_jobs_continuous.py

+        of ratings across all explorations and average rating.
+
+        Args:
+            key: str. The unique id of user


Add period at end; ditto elsewhere.

seanlip · 2017-06-24T23:03:27Z

core/domain/user_jobs_continuous.py

@@ -520,6 +594,17 @@ def map(item):

    @staticmethod
    def reduce(key, stringified_values):
+        """Implements the reduce function.
+        This function creates or updates the UserStatsModel instance for the given 


Add newline above this one (since it's a new paragraph).

codecov-io · 2017-07-01T11:35:23Z

Codecov Report

Merging #3569 into develop will decrease coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #3569      +/-   ##
===========================================
- Coverage    44.85%   44.84%   -0.01%     
===========================================
  Files          257      257              
  Lines        19871    19873       +2     
  Branches      3131     3132       +1     
===========================================
  Hits          8913     8913              
- Misses       10958    10960       +2

Impacted Files	Coverage Δ
...ev/head/pages/exploration_editor/EditorServices.js	`35.13% <0%> (-0.07%)`	⬇️

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 f904ee9...b9c21be. Read the comment docs.

seanlip

Thanks @shubha1593! This is looking great; I've taken a first pass and made a few more notes, but they're mostly on small things. PTAL?

seanlip · 2017-07-02T03:39:38Z

core/domain/user_jobs_continuous.py

+
+        Returns:
+            tuple(float, list(dict)). A 2-tuple with following entries:
+            -float. The time, in milliseconds since the Epoch, when the job was queued.


Nit: here and below, please add a single space after each "-".

seanlip · 2017-07-02T03:39:51Z

core/domain/user_jobs_continuous.py

+                -'activity_id': str. The ID of the exploration being committed to or
+                 to which the feedback thread belongs.
+                -'activity_title': str. The title of the activity.
+                -'last_updated_ms': float. The time when the update was made. 


The time when the update was made, in milliseconds since the Epoch.

seanlip · 2017-07-02T03:42:00Z

core/domain/user_jobs_continuous.py

+                -'activity_title': str. The title of the activity.
+                -'last_updated_ms': float. The time when the update was made. 
+                -'author_id': str. The ID of the author who made the update.
+                -'subject': str. The commit message or message indicating a feedback 


I think you can say: "A brief description of the notification." This automatically covers cases where additional notification types are added in the future.

seanlip · 2017-07-02T03:44:52Z

core/domain/user_jobs_continuous.py

-                      was deleted.
-                - A list containing valid activity model instances which are
-                  mappable to feedback threads
+            tuple(list(dict), list(*)). A 2-tuple with following entries:


Don't the elements of the second list have a specific type?

tuple(list(dict), list(ExplorationModel|...|...)).

(where you should enumerate the different classes that can occur in the second argument).

Okay, do you want me to specify all the models in the argument ?
('activity', 'audit', 'base_model', 'classifier', 'collection', 'config',
'email', 'exploration', 'feedback', 'file', 'job', 'recommendations',
'statistics', 'user')

Nope, just the specific model classes that are processed in this job and that can appear in this tuple.

seanlip · 2017-07-02T03:45:16Z

core/domain/user_jobs_continuous.py

-                  mappable to feedback threads
+            tuple(list(dict), list(*)). A 2-tuple with following entries:
+                -list(dict): A list, having information for every activity in 
+                    activity_ids_list. Dict have the following keys:


Dict have --> Each dict in this list has

seanlip · 2017-07-02T04:01:03Z

core/domain/user_jobs_continuous.py

-        - 'total_plays': # of times the user's explorations were played
-        - 'num_ratings': # of times the user's explorations were started
-        - 'average_ratings': average of average ratings across all explorations
+            'total_plays': int. # of times the user's explorations were played.


--> Number; ditto in next line.

seanlip · 2017-07-02T04:01:43Z

core/domain/user_jobs_continuous.py

+
+        Yields:
+            This function may yield as many times as appropriate 2-tuples in the 
+            format: (str, dict) where


Ditto (see a previous comment) re punctuation.

seanlip · 2017-07-02T04:01:58Z

core/domain/user_jobs_continuous.py

+            -dict: A dict that includes entries for all the explorations that this
+                user contributes to or owns. Each entry has the following keys:
+                - 'exploration_impact_score': float. The impact score of all the
+                 explorations contributed to by the user.


This should be indented by 4 from the previous line. Similarly below.

seanlip · 2017-07-02T04:03:17Z

core/domain/user_jobs_continuous.py

+
+        Args:
+            key: str. The unique ID of the user.
+            stringified_values: list(str(dict)). A list of information regarding all


See previous comment re type info.

seanlip · 2017-07-02T04:03:26Z

core/domain/user_jobs_continuous.py

+                the explorations that this user contributes to or owns. Each entry is
+                a stringified dict having the following keys:
+                - 'exploration_impact_score': float. The impact score of all the
+                 explorations contributed to by the user.


Ditto re indentation.

… (address review comments)

shubha1593 · 2017-07-05T06:11:59Z

@seanlip made the modifications, please have a look.

seanlip

This looks excellent, LGTM. Thank you, @shubha1593!

seanlip · 2017-07-05T08:48:38Z

Or rather ... almost excellent. I'm afraid the lint checks fail; please take a look at the logs. (Did the lint checks not run automaticaly before you pushed?)

Assigning this to @AllanYangZhou to merge once the lint checks are fixed.

AllanYangZhou · 2017-07-07T19:03:36Z

Hi @shubha1593, have you had a chance to look at the lint errors and correct them? Looks like the only issue is that some lines are too long, once that's fixed I can merge it in.

shubha1593 · 2017-07-08T00:44:40Z

Hi @AllanYangZhou, I am having issues with my machine, need to format it. I will fix the lint errors by today or tomorrow definitely.

AllanYangZhou · 2017-07-09T00:54:01Z

Understood, thanks!

shubha1593 · 2017-07-09T13:23:34Z

Hi @AllanYangZhou, Fixed the lint errors.

AllanYangZhou · 2017-07-09T16:24:49Z

Nice, thanks!

…date * upstream/develop: (32 commits) Remove erroneous clause in app.yaml. (oppia#3643) Part of oppia#2447: Upgrade select2 library (oppia#3626) Fix part of oppia#2394: Added docstrings in exp_domain.py (oppia#3578) Fix oppia#3618: stop icon overlap (oppia#3642) Add backend functionality to correctly handle needs_update part of subtitled HTML. (oppia#3620) Change 'style' to 'ng-attr-style' in an attempt to fix IE bug in collection viewer. (oppia#3636) Fix typo in Exploration editor (oppia#3637) Update credits (oppia#3633) Fix oppia#2639: Updated Google chart/visualization Library loader code and use async to load the library (oppia#3614) Fix user bios job in the case that user_bio is None (oppia#3632) Update changelog and credits for release 2.5.2 (oppia#3628) Fix oppia#2394: Add docstring to core.domain.user_jobs_continous.py (oppia#3569) Fix oppia#3585: Corrected arrows in exploration progress nav (oppia#3605) change the text edit error message Corrected tense of error message Corrected state to read card Close modal window if clicked outside (oppia#3623) Storage, Domain classes and helper functions for ClassifierExplorationMapping (oppia#3583) removed super admin role from admin interface. (oppia#3621) ...

shubha gupta added 4 commits June 23, 2017 21:46

Add docstrings to core.domain.user_jobs_continuous.py

b2f4a1f

Fix oppia#2394: Add docstrings to core.domain.user_jobs_continuous.py

e51c321

Merge remote-tracking branch 'upstream/develop' into develop

036fb37

Merge branch 'develop' into update-docstring

b0ce63d

shubha1593 requested a review from seanlip June 24, 2017 06:05

seanlip requested changes Jun 24, 2017

View reviewed changes

shubha gupta added 2 commits July 1, 2017 17:00

Merge remote-tracking branch 'upstream/develop' into update-docstring

dae0baa

Address review comments

ab87295

shubha1593 requested a review from seanlip July 1, 2017 15:04

seanlip requested changes Jul 2, 2017

View reviewed changes

shubha gupta added 2 commits July 5, 2017 11:30

Merge remote-tracking branch 'upstream/develop' into update-docstring

6592891

Fix oppia#2394: Add docstrings to core.domain.user_jobs_continuous.py…

1ef1c43

… (address review comments)

shubha1593 requested a review from seanlip July 5, 2017 06:09

seanlip approved these changes Jul 5, 2017

View reviewed changes

seanlip added the PR: LGTM label Jul 5, 2017

seanlip assigned AllanYangZhou Jul 5, 2017

Merge remote-tracking branch 'upstream/develop' into update-docstring

b9c21be

AllanYangZhou merged commit 8aab2a4 into oppia:develop Jul 9, 2017

shubha1593 deleted the update-docstring branch July 9, 2017 16:51

Fix #2394: Add docstring to core.domain.user_jobs_continous.py #3569

Fix #2394: Add docstring to core.domain.user_jobs_continous.py #3569

Conversation

shubha1593 commented Jun 24, 2017

seanlip 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

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

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

codecov-io commented Jul 1, 2017 • edited Loading

Codecov Report

seanlip 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

Choose a reason for hiding this comment

--> Number; ditto in next line.

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

shubha1593 commented Jul 5, 2017

seanlip left a comment

Choose a reason for hiding this comment

seanlip commented Jul 5, 2017

AllanYangZhou commented Jul 7, 2017

shubha1593 commented Jul 8, 2017

AllanYangZhou commented Jul 9, 2017

shubha1593 commented Jul 9, 2017

AllanYangZhou commented Jul 9, 2017

codecov-io commented Jul 1, 2017 •

edited

Loading