Model discrete sampling geometry datasets #62

rod-glover · 2018-03-02T23:23:40Z

Resolves #60

Includes Alembic schema+data migration. Both upgrade and downgrade have been tested on a SQLite db but not a Postgres db.
Modifies indexer and tests to work with new ORM/schema, but only for gridded datasets, which were the kind handled originally. Indexing DSG TS datasets will be the subject of another PR.
Cleans up and PEP8-ifies v2 ORM code, which was a little untidy.
Adds tests specifically for new, polymorphic ORM for discrete sampling and gridded sampling geometries. These may seem a little superfluous, but they are a way of ensuring that we correctly understand how SQLAlchemy handles polymorphism/inheritance. Turns out it's pretty straightforward.

TODO:

In database migration, either
- remove all DataFile.*_dim_name attributes, or
- make them all nullable (some aren't) so that they can be nulled out for DSG TS files, to which these attrs are irrelevant.
? Test migration against a Postgres database? Can we (Matthew, heh) make a copy of ce_meta easily? That would be best.

rod-glover · 2018-03-02T23:40:33Z

alembic/versions/12f290b63791_handle_variant_sampling_geometries.py

+
+def downgrade():
+    # This downgrade will dump all discrete geometry data.
+    # TODO: Add user confirmation of downgrade?


@jameshiebert what do you think?

rod-glover · 2018-03-02T23:42:51Z

tests/modelmeta/test_orm.py

+    assert variable_alias_1.data_file_variables == [dfv_dsg_time_series_1]
+
+
+def test_station(test_session_with_empty_db, station_1):


Remove this test. It's too simple.

jameshiebert

Looks well thought out to me. Admittedly, have the PEP8 fixes mixed in with substantive changes is a little distracting. Though I appreciate you having done it, please separate those out next time we run into this.

Both upgrade and downgrade have been tested on a SQLite db but not a Postgres db

I thought that we're using testing.postgresql on this repo...

jameshiebert · 2018-03-05T18:42:02Z

modelmeta/v2.py


    def __repr__(self):
-        return '{}({}, {}, {}, {})'.format(self.__class__.__name__,
-                self.time_idx, self.time_end, self.time_start, self.time_set_id)
+        return obj_repr('time_set_id time_idx time_start time_end', self)


Yeah, that's handy.

jameshiebert · 2018-03-05T18:44:22Z

modelmeta/v2.py

+            'netcdf_variable_name disabled range_min range_max', self)
+
+
+class DataFileVariableDSGTimeSeries(DataFileVariable):


Could probably use a docstring here to elaborate on what DSG stands for.

rod-glover · 2018-03-05T20:11:20Z

Yes, sorry about mixing in the PEP-8 fixup. I can remove it if you like and put it in a separate PR. Shouldn't be very difficult. A product of Friday-night bleariness.
So far, migrations don't have unit tests. The reason is that within a PR, the ORM matches the final migrated state of the database, not an earlier state. The unit tests (relying on testing.postgresql) just create the database from the (current) ORM. There are a couple of ways we could create a unit test environment for the upgrade migrations:
1. Stash an old version of the ORM, create a test database based on that, populate it with some test data, migrate, run unit tests.
2. Populate and dump a Postgres database in the old state, load it up in, migrate, run unit tests.
3. Both of those solutions have always seemed fairly effortful to me, and so I rely on testing against local databases, whether SQLite or Postgres. I am currently making a copy of ce_meta locally so that I have something safe to test against.
For the downgrade migrations, it's easier, since you're downgrading from a latest version of the ORM.

rod-glover · 2018-03-05T20:15:25Z

OK, embarassing, there's this. I'll look into it and any similar packages.

jameshiebert · 2018-03-05T20:31:54Z

OK, embarassing, there's this. I'll look into it and any similar packages.

Wouldn't call that embarrassing. I didn't know about that, and it looks like a good resource. To boot, I hadn't thought through the fact that to test the database, your initial instance will be the current ORM schema. In any case, checking out alembic-verify sounds like a good path.

I can remove it if you like and put [the PEP8 migration] in a separate PR.

No don't worry about it. Just keep it in mind for next time.

jameshiebert · 2018-03-07T20:36:25Z

LGTM. Are you happy with this at present @rod-glover ?

rod-glover · 2018-03-08T17:44:04Z

@jameshiebert : One small addition: Migration now makes data_files.x_dim_name, .y_dim_name nullable so that we don't have to fill in nonsense values for them for DSG data files. We didn't really make a decision whether or not to remove these (I'd like to), so this seemed the prudent choice at the moment. We can migrate those columns out later if we decide to. I'll put in an issue about that.

I am happy with this now and ready to merge if you are.

rod-glover added 15 commits March 2, 2018 15:11

Modify ORM to accommodate gridded and discrete sampling geometries

5bddbc3

Add basic tests for modified elements of ORM

33d166c

Move existing modelmeta ORM tests into tests/modelmeta

26602d1

Rename db class diagram

6c7b406

Update ORM class diagram

91a5ceb

Delete junk

4547f76

Add relations to DataFileVariableDSGTimeSeriesXStation

26bdbb7

Add tests for cascading deletes on cross table

bdda1d7

Clean up ORM code and add __repr__ for each class

afc9e27

Add cascading deletes for ORM child classes

62f1142

Tweak ORM definitions

dfa1566

Update test_index tests to work with new model

7f7e0f0

Create Alembic schema and data migration for new model

751be0a

Upgrade test SQLite database to new model

36710e3

Fix tests against SQLite test db (new model)

f24976e

rod-glover requested a review from jameshiebert March 2, 2018 23:29

rod-glover commented Mar 2, 2018

View reviewed changes

jameshiebert reviewed Mar 5, 2018

View reviewed changes

rod-glover added 8 commits March 5, 2018 16:31

Add alembic-verify to test requirements

5447240

Add initial alembic-verify test

fef936d

Modify env.py so it works in the alembic-verify test environment

2d5a297

Add 'create extension postgis' to initial create migration

259b1cd

Fix a bug (!) in add_seasonal... migration

8646523

Make handle_variant... migration work in alembic-verify test environment

84733e6

Fix migration so that it doesn't make geometry_type default to 'gridded'

1954a0f

Add model-vs-migration schema tests

c3a270b

rod-glover added 7 commits March 6, 2018 08:49

Add alembic to test requirements

019de40

Add data migration tests

ee321dd

Modify alembic_root

e031380

Code cleanup

d2c84f3

Fix weird character in comments that breaks Py 2.7

404791f

Use explicit rev id for downgrade test

27c0b5d

Add coding declaration to problematic file

02b1e56

rod-glover added 3 commits March 7, 2018 16:03

Remove unnecessary test

dbebe06

Add some docstrings to ORM declaration

8cccda9

Migration: make data_files.x_dim_name, .y_dim_name nullable in upgrade

aa3ec9e

jameshiebert merged commit 673213a into master Mar 8, 2018

jameshiebert deleted the i/60-discrete-geometry branch March 8, 2018 21:10

rod-glover mentioned this pull request Jul 3, 2018

Use modelmeta database version 12f290b63791 pacificclimate/climate-explorer-backend#81

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Model discrete sampling geometry datasets #62

Model discrete sampling geometry datasets #62

rod-glover commented Mar 2, 2018 •

edited

Loading

rod-glover Mar 2, 2018

rod-glover Mar 2, 2018

jameshiebert left a comment

jameshiebert Mar 5, 2018

jameshiebert Mar 5, 2018

rod-glover commented Mar 5, 2018 •

edited

Loading

rod-glover commented Mar 5, 2018

jameshiebert commented Mar 5, 2018

jameshiebert commented Mar 7, 2018

rod-glover commented Mar 8, 2018

		assert variable_alias_1.data_file_variables == [dfv_dsg_time_series_1]


		def test_station(test_session_with_empty_db, station_1):

		'netcdf_variable_name disabled range_min range_max', self)


		class DataFileVariableDSGTimeSeries(DataFileVariable):

Model discrete sampling geometry datasets #62

Model discrete sampling geometry datasets #62

Conversation

rod-glover commented Mar 2, 2018 • edited Loading

rod-glover Mar 2, 2018

Choose a reason for hiding this comment

rod-glover Mar 2, 2018

Choose a reason for hiding this comment

jameshiebert left a comment

Choose a reason for hiding this comment

jameshiebert Mar 5, 2018

Choose a reason for hiding this comment

jameshiebert Mar 5, 2018

Choose a reason for hiding this comment

rod-glover commented Mar 5, 2018 • edited Loading

rod-glover commented Mar 5, 2018

jameshiebert commented Mar 5, 2018

jameshiebert commented Mar 7, 2018

rod-glover commented Mar 8, 2018

rod-glover commented Mar 2, 2018 •

edited

Loading

rod-glover commented Mar 5, 2018 •

edited

Loading