Export and merge functions #17

ajparsons · 2017-02-06T16:31:19Z

Repackaged version of the export feature branch - split into more commits to separate different features and refactoring. Incorporating changes made to package since December.

Now also includes a feature for intelligent merging of similar popolo (where there will be overlap of individuals - but one file might have additional memberships).

Includes workaround for no to_string function in approx_date package - this logic also in mysociety/approx_dates#2 to approx_dates.

Tests written for export + merge features.

…e.py The new attribute system will involve meta classes other helper objects I prefer to keep separate from the objects that use them for neatness.

Rather than specify which data model the collection is a 'collection' of in an overridden __init__ - now subclasses override the object_class class attribute with the correct data model. Not hugely important, stylistic choice - prefer avoiding super calls.

Previously a MembershipCollection object would be generated on request on accessing the membership property. Now this is generated and assigned at creation. Reason: collections need to be permanent so that changes to their objects can be repackaged into a file.

In new system, rather than the properties of a PopoloObject being defined as @property-ed functions accessing properties of self.data - this will be replaced by the 'Attribute' class - that given a attribute will control getting and setting to self.data. Subclasses of Attribute control different ways of talking to data e.g. talking to data that is a date. A small meta-class is used for PopoloObject to avoid repetition in definitions. e.g. person = Property(attr=person) can be expressed as person = Property()

Almost all previous @property-ed functions replaced with Attribute object. Functionality for 'getting' and default values are identical to old arrangement.

When a new 'twitter' value is set, if a url - the link value is set - and the username is also extracted for contact details. If not a url, just updates the contact details.

Date fields for Person and Organization were set to default to None rather than the vague .PAST and .FUTURE from approxdate. This has been changed to bring it into sync with other Membership and Event objects. Person Test updated to reflect this change.

Popolo objects can now export to json (to_json) or direct to a file name.

Two popolo objects can now be merged. Each collectiontype has a value that must be made 'unique' (either name or id). In a collision, the 'larger' will be kept (assumed more information) and all refs to discarded ids replaced. More complex functionality e.g. the bigger person object picking up all possible alternate names of the smaller one, can be specified by overriding the absorb function.

Datetime objects will be reduced to just their date, then converted to isoformat

Exposed json_data as something that can be accessed, but that is not independent of child objects. New objects are based on a deepcopy of the incoming dict.

Updated person tests with test for __gt__ and __lt__

…ssigning 'none'.

…dAttributes Originally this was so you could build RelatedAttributes on something other than an ID field. This was never necessary, so removed to tidy up.

These features will also be useful in the approx_dates package directly, and will be ported there at some point.

coveralls · 2017-02-06T16:34:02Z

Coverage remained the same at 100.0% when pulling 37b644c on ajparsons:export_merge_function into 21670d6 on everypolitician:master.

Returns either unique time from membership or that from overall legislative period.

Nicer way of getting a wikidata, parlparse, etc for an individual.

Replaces with a cross-platform compatible hashing method so membership ids are generated consistently

Will absorb gender of 'other' when self has none.

houndci-bot · 2018-01-31T13:56:21Z

tests/test_save.py

+        pop.to_filename(filename)
+        os.remove(filename)
+
+


blank line at end of file
blank line contains whitespace

houndci-bot · 2018-01-31T13:56:21Z

tests/test_save.py

+        filename = mktemp()
+        pop.to_filename(filename)
+        os.remove(filename)
+


blank line contains whitespace

houndci-bot · 2018-01-31T13:56:21Z

tests/test_save.py

+        assert pop2.to_json() == pop.to_json()
+
+        assert pop.memberships[0].id == pop2.memberships[0].id
+


blank line contains whitespace

houndci-bot · 2018-01-31T13:56:21Z

tests/test_save.py

+        j = pop.to_json()
+        pop2 = Popolo(json.loads(j))
+        assert pop2.to_json() == pop.to_json()
+


blank line contains whitespace

houndci-bot · 2018-01-31T13:56:21Z

tests/test_save.py

+        pop.add(m)        
+
+
+        """


too many blank lines (2)

houndci-bot · 2018-01-31T13:56:22Z

tests/test_save.py

+        membership_attributes = {"role":"MP",
+                                 "person_id":"person1",
+                                 "organisation_id":"org1",
+                                 "area_id":"area1",


missing whitespace after ':'

houndci-bot · 2018-01-31T13:56:22Z

tests/test_save.py

+
+        membership_attributes = {"role":"MP",
+                                 "person_id":"person1",
+                                 "organisation_id":"org1",


missing whitespace after ':'

houndci-bot · 2018-01-31T13:56:23Z

tests/test_save.py

+        pop.add(e)
+
+        membership_attributes = {"role":"MP",
+                                 "person_id":"person1",


missing whitespace after ':'

houndci-bot · 2018-01-31T13:56:23Z

tests/test_save.py

+            assert getattr(e, k) == v
+        pop.add(e)
+
+        membership_attributes = {"role":"MP",


missing whitespace after ':'

houndci-bot · 2018-01-31T13:56:23Z

tests/test_save.py

+            setattr(e, k, v)
+            assert getattr(e, k) == v
+        pop.add(e)
+


blank line contains whitespace

mhl

Hi @ajparsons - I think I'm OK with these changes in principle, but there are few things that make this hard to review for me. I've added HoundCI now, who's automatically commented on the not very interesting stylistic things. I've also left some line comments (although GitHub showing some of these as "outdated" now after I've rebased).

A key thing is that I'd like the tests to pass at each commit; I mentioned this particularly in the case of a commit that should have just been a refactoring, but caused the tests to start failing. A nice trick for checking that this is the case is to use:

git rebase -i --exec 'tox' master

So basically, I like the direction this is heading in, but I'd like to come back to it once some of these things are sorted out.

mhl · 2018-01-31T12:11:56Z

.gitignore

@@ -7,3 +7,6 @@
 /htmlcov
 /everypolitician_popolo.egg-info
 /dist
+.project


I think generally I'd expect people to put ignore rules that are specific to a particular IDE into .git/info/exclude (or to have them in a per-user gitignore configured with: git config --global core.excludesfile $HOME/.gitignore. (I'm happy with adding the .gitattributes line-endings configuration, though.)

mhl · 2018-01-31T12:22:52Z

popolo_data/collections.py

        self.all_popolo = all_popolo
-        self.object_class = object_class
+        self.object_class = self.__class__.object_class


I don't think this line is actually necessary, is it? self.foo falls back to trying to find a class variable foo, e.g.:

In [1]: class Thing(object): ...: foo = 'class foo' ...: bar = 'class bar' ...: def __init__(self): ...: self.foo = 'instance foo' ...: def print_stuff(self): ...: print self.foo ...: print self.bar ...: In [2]: Thing().print_stuff() instance foo class bar

mhl · 2018-01-31T12:35:23Z

popolo_data/base.py

@@ -4,44 +4,20 @@


 from approx_dates.models import ApproxDate
-from six.moves.urllib_parse import urlsplit


With a refactoring like this commit, I'd really like the tests to still pass at this point, but the imports in other modules haven't been updated and the tests don't pass at this point, so I think the commit is incomplete. (That would also make some of the later commits easier to read.)

mhl · 2018-01-31T12:54:57Z

popolo_data/importer.py


+    def __init__(self, json_data=None):
+        if json_data == None:


Tiny stylistic thing - since None is a singleton, PEP 8 suggests if json_data is None:

mhl · 2018-01-31T12:56:53Z

popolo_data/base.py

@@ -1,30 +1,180 @@
-from datetime import date


There are some typos in this commit message (e.g. "In new system", "a attribute", extra space before "to self.data"

mhl · 2018-01-31T12:57:50Z

popolo_data/base.py

+    will return None rather than an error if the value is absent.
+
+    '''
+    def __init__(self,attr="",default=None,null=False,allow_multiple=False):


There should be spaces after the commas here (we really should have an automated check for this kind of thing)

mhl · 2018-01-31T12:59:49Z

popolo_data/base.py

+        self.allow_multiple = allow_multiple
+
+    def __get__(self, obj, type=None):
+        if self.allow_null_default == False and self.default_value == None:


Couldn't this be if (not self.allow_null_default) and (self.default_value is None): ?

ajparsons added 18 commits February 3, 2017 12:38

Amending project attributes for use in eclipse development enviroment

2dbc7cf

Split collections and item classes into separate files and out of bas…

2014840

…e.py The new attribute system will involve meta classes other helper objects I prefer to keep separate from the objects that use them for neatness.

PopoloObject subclasses moved to new attribute system

bc36884

Almost all previous @property-ed functions replaced with Attribute object. Functionality for 'getting' and default values are identical to old arrangement.

Created 'twitter' setter for Person object.

2cbccd5

When a new 'twitter' value is set, if a url - the link value is set - and the username is also extracted for contact details. If not a url, just updates the contact details.

Added export functionality

240d637

Popolo objects can now export to json (to_json) or direct to a file name.

Amended handling of default settings.

d6c7d73

Date attributes will now accept datetime objects being 'set'

d5366c5

Datetime objects will be reduced to just their date, then converted to isoformat

Add safeguards to prevent objects sharing attributes.

400691f

Exposed json_data as something that can be accessed, but that is not independent of child objects. New objects are based on a deepcopy of the incoming dict.

Added test suite with tests for property setting and file saving.

7b6166a

Updated person tests with test for __gt__ and __lt__

Assigning a null contact_detail will now delete the row rather than a…

2520cfd

…ssigning 'none'.

Removed unused ability to specify which field is used to match Relate…

461cedf

…dAttributes Originally this was so you could build RelatedAttributes on something other than an ID field. This was never necessary, so removed to tidy up.

Added processing for handling of saving vague-dates and date ranges.

63a87e9

These features will also be useful in the approx_dates package directly, and will be ported there at some point.

Python 3 Compatibility changes.

270f68a

ajparsons mentioned this pull request Feb 6, 2017

[WIP] Export functionality #16

Closed

ajparsons added 4 commits January 30, 2018 13:36

Created effective_start_date and effective_end_date properties

04a8729

Returns either unique time from membership or that from overall legislative period.

Add get_identfier function

c4a39a5

Nicer way of getting a wikidata, parlparse, etc for an individual.

Updated hash id system for memberships

ba755f7

Replaces with a cross-platform compatible hashing method so membership ids are generated consistently

Fix to merge function

6df1ea7

Will absorb gender of 'other' when self has none.

ajparsons force-pushed the export_merge_function branch from c718cf5 to 6df1ea7 Compare January 31, 2018 09:29

mhl force-pushed the export_merge_function branch from 6df1ea7 to 37b644c Compare January 31, 2018 13:56

houndci-bot reviewed Jan 31, 2018

View reviewed changes

mhl suggested changes Jan 31, 2018

View reviewed changes

ajparsons added 2 commits January 22, 2020 15:27

Allowed source as an attribute of membership

7db9f3c

changed default loading to uft-8 encoding

ccbd20d

ajparsons force-pushed the export_merge_function branch from 37b644c to ccbd20d Compare January 22, 2020 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Export and merge functions #17

Export and merge functions #17

ajparsons commented Feb 6, 2017

coveralls commented Feb 6, 2017 •

edited

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

houndci-bot Jan 31, 2018

mhl left a comment

mhl Jan 31, 2018

mhl Jan 31, 2018

mhl Jan 31, 2018

mhl Jan 31, 2018

mhl Jan 31, 2018

mhl Jan 31, 2018

mhl Jan 31, 2018

		assert pop2.to_json() == pop.to_json()

		assert pop.memberships[0].id == pop2.memberships[0].id

		@@ -4,44 +4,20 @@


		from approx_dates.models import ApproxDate
		from six.moves.urllib_parse import urlsplit

Export and merge functions #17

Are you sure you want to change the base?

Export and merge functions #17

Conversation

ajparsons commented Feb 6, 2017

coveralls commented Feb 6, 2017 • edited

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

mhl 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

coveralls commented Feb 6, 2017 •

edited