Issue1 gcmd short names #52

mortenwh · 2019-02-06T12:08:49Z

… geospaas.viewer.tests

…ularies.models

…st version of Aqua platform keywords (was AQUA before)

…from ForeignKey source to ManyToManyField sources, changed __str__ method of Source to rely on platform and instrument only (not specs) and removed the field specs.

…atform

…o allow multiple sources

…de itself. Assert that the entry_title is now equal to the filename, test new use of 'platform/instrument' as list

…in the natural_keys

…t and parameter instead of source. This is still ongoing..

… some old commented code

… there is extra information linked to the connection between two models. In this case, we don't have that...

…new migrations, and updated fixtures. This also applies to #51

…tests to accomodate model changes

…es field

…d search in viewer.views.IndexView.render

…he Django server in the local web browser

coveralls · 2019-02-06T12:15:56Z

Pull Request Test Coverage Report for Build 202

270 of 284 (95.07%) changed or added relevant lines in 21 files are covered.
14 unchanged lines in 2 files lost coverage.
Overall coverage increased (+1.2%) to 93.614%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
geospaas/catalog/migrations/0007b_migrate_source_data.py	8	14	57.14%
geospaas/catalog/migrations/0008b_migrate_parameters_data.py	9	17	52.94%

Files with Coverage Reduction	New Missed Lines	%
geospaas/vocabularies/tests.py	6	97.93%
geospaas/vocabularies/managers.py	8	89.03%

Totals
Change from base Build 171:	1.2%
Covered Lines:	1554
Relevant Lines:	1660

💛 - Coveralls

…ged natural_key and get_by_natural_key methods of the Source model

…pt as described in #55

mortenwh · 2019-02-06T15:42:35Z

I'm not sure how to test the data migrations automatically (i.e., geospaas/catalog/migrations/0007b_migrate_source_data.py and
geospaas/catalog/migrations/0008b_migrate_parameters_data.py) but this was done manually as described in #1.

akorosov

Looks ok. Just few things:

Too many migrations - maybe some can be squashed?
Too verbose output for some vocabularies. Is it really necessary? Looks like it kills the usability of __str__ method (That was not in the original issue, BTW. We should try to make PRs small and focused on the issues only).
Dosn't matter since we swtich to Docker soon (hopefully) but it is better not to provide IP in Vagrantfile and ansible as it may conflict with other VMs or containers.

akorosov · 2019-02-14T13:21:12Z

Vagrantfile

@@ -10,6 +10,7 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
  config.vm.box_url = "https://atlas.hashicorp.com/ubuntu/trusty64"

  config.vm.define "geospaas", primary: true do |geospaas|
+    geospaas.vm.network :private_network, ip: "192.168.33.10"


This is not needed. In fact it can conflict with other VMs for other projects. If not specified, vagrant allocates free IP to the VM without conflicts.

akorosov · 2019-02-14T13:23:35Z

geospaas/catalog/migrations/0008a_add_newparameters_field.py

+    operations = [
+        migrations.AddField(
+            model_name='dataset',
+            name='newparameters',


Can you squash these migrations? You added newparameters and then removed it but generated migrations in between.

akorosov · 2019-02-14T13:30:13Z

geospaas/catalog/models.py

@@ -131,13 +128,14 @@ class Dataset(models.Model):
        ]
    )
    entry_title = models.CharField(max_length=220)
-    parameters = models.ManyToManyField(Parameter, through='DatasetParameter')
+    #parameters = models.ManyToManyField(Parameter, through='DatasetParameter')
+    parameters = models.ManyToManyField(Parameter)


Can you also remove the model DatasetParameter if it is not used anymore?

akorosov · 2019-02-14T13:47:20Z

geospaas/catalog/tests.py

+
+    def test_source__str__method(self):
+        # Assure __str__ method returns correct string
+        expected_str = 'Platform: (Category: Earth Observation Satellites, Series Entity: , Short Name: ' \


That looks terrible. IMHO now Source.__str__ is not usable.

akorosov · 2019-02-14T13:55:21Z

geospaas/vocabularies/models.py

@@ -42,10 +44,13 @@ class Instrument(models.Model):
    objects = InstrumentManager()

    def __str__(self):
-        return str(self.short_name)
+        return 'Category: %s, Instrument Class: %s, Type: %s, Subtype: %s, Short Name: %s, ' \


Why do we need that? What is the workflow? It looks too verbose but adds no information.

mortenwh · 2019-02-24T09:54:26Z

This bug was in issue40_thredds_crawler - needs to be checked as well...

    149     def __str__(self):
    150         return '%s/%s/%s' % (self.source.platform, self.source.instrument,
--> 151                 self.time_coverage_start.isoformat())
    152 
    153 # Keep this for reference if we want to add it

AttributeError: 'str' object has no attribute 'isoformat'

akorosov · 2020-03-30T11:34:15Z

A much simple solution was proposed in #80.

mortenwh added 30 commits February 1, 2019 14:52

#1: deleted fixtures not used in the tests. New tests use mock.

c892f8a

#1: Some more fixtures that were needed in geospaas.catalog.tests and…

ff12aec

… geospaas.viewer.tests

#1: added an entry to self.mock_pti.get_gcmd_platform_list

61ae0ec

#1: Added fields to __str__ and natural_key methods in geospaas.vocab…

b145dfa

…ularies.models

#1: Updated catalog fixtures to new source model, and changed to late…

d894b51

…st version of Aqua platform keywords (was AQUA before)

#1: Changed __str__ method of Dataset to return entry_title, changed …

9ad8686

…from ForeignKey source to ManyToManyField sources, changed __str__ method of Source to rely on platform and instrument only (not specs) and removed the field specs.

#1: Test updated to use Aqua instead of AQUA as short name for the pl…

dabc835

…atform

#1: Set entry_title to the filename of the ingested dataset, update t…

c8c020f

…o allow multiple sources

#1: Do not raise error in the tests, this should be handled by the co…

3f90889

…de itself. Assert that the entry_title is now equal to the filename, test new use of 'platform/instrument' as list

#1: new name for Aqua platform (was AQUA before)

52cbc74

#1: New methods get_by_natural_key to accomodate for more parameters …

09dee8d

…in the natural_keys

#1: use only short_name in the __str__ method of DataCenter

4cc45ee

#1: Updating viewer Search model and forms to use platform, instrumen…

ad7bd0b

…t and parameter instead of source. This is still ongoing..

#1: new migrations to accomodate model changes

b266add

#1: Added parameters to fixture

d0d7eb4

#1: viewer form working with platform and instrument instead of source

ed2eb04

#1: search platform, instrument and parameter in views.py and removed…

e38423b

… some old commented code

#1: removed intermediate models. These should not be necessary unless…

eb4e9ea

… there is extra information linked to the connection between two models. In this case, we don't have that...

#1: Removed intermediate relationship through DatasetParameter, made …

ef136a7

…new migrations, and updated fixtures. This also applies to #51

#1: merged with origin and resolved conflicts

4b97b52

#1: Removed old commented code

8a15d6c

#1: viewer tests passing

c88c01d

#1: import nansat_filename in test..

2e05634

#1: removed duplicate parameter entry in fixture and updated catalog.…

6e853d4

…tests to accomodate model changes

#1:renamed migration file

b3ff98b

#1: Added data migration of Dataset.source field to new Dataset.sourc…

6208a4b

…es field

#1: corrected app label in migration file

ebe676a

#1: ongoing changes to update contents of Dataset.parameters field

a83c862

#1: data migration for parameters field

ee30d85

#1: both forward and backward migrations working

78d0ca1

mortenwh added 2 commits February 5, 2019 17:02

#1: Added default None in search model, created migration, and update…

3e44f5b

…d search in viewer.views.IndexView.render

#1: Assigned IP-address to the vm to be able to see the output from t…

7d6a45d

…he Django server in the local web browser

mortenwh requested a review from akorosov February 6, 2019 12:08

mortenwh added 6 commits February 6, 2019 15:29

#1: changed __str__ methods of Source and DatasetURI models, and chan…

1401922

…ged natural_key and get_by_natural_key methods of the Source model

#1: full test coverage of catalog.models

8fe4d11

#1: reinserted fixture..

822a589

#1: almost full test coverage of geospaas.vocabularies.managers, exce…

ada572d

…pt as described in #55

#1: test coverage of new parameter search

8be4bd2

#1: test coverage of new parameter search in viewer.views.py

03ada4d

mortenwh added 2 commits February 6, 2019 17:52

#1: make sure natural_key methods return tuple

37048b1

#1: full test coverage of vocabularies/models.py

e5b22b5

akorosov requested changes Feb 14, 2019

View reviewed changes

akorosov closed this Mar 30, 2020

akorosov deleted the issue1_gcmd_short_names branch March 30, 2020 11:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue1 gcmd short names #52

Issue1 gcmd short names #52

mortenwh commented Feb 6, 2019

coveralls commented Feb 6, 2019 •

edited

Loading

mortenwh commented Feb 6, 2019

akorosov left a comment

akorosov Feb 14, 2019

akorosov Feb 14, 2019

akorosov Feb 14, 2019

akorosov Feb 14, 2019

akorosov Feb 14, 2019

mortenwh commented Feb 24, 2019 •

edited

Loading

akorosov commented Mar 30, 2020

Issue1 gcmd short names #52

Issue1 gcmd short names #52

Conversation

mortenwh commented Feb 6, 2019

coveralls commented Feb 6, 2019 • edited Loading

Pull Request Test Coverage Report for Build 202

💛 - Coveralls

mortenwh commented Feb 6, 2019

akorosov left a comment

Choose a reason for hiding this comment

akorosov Feb 14, 2019

Choose a reason for hiding this comment

akorosov Feb 14, 2019

Choose a reason for hiding this comment

akorosov Feb 14, 2019

Choose a reason for hiding this comment

akorosov Feb 14, 2019

Choose a reason for hiding this comment

akorosov Feb 14, 2019

Choose a reason for hiding this comment

mortenwh commented Feb 24, 2019 • edited Loading

akorosov commented Mar 30, 2020

coveralls commented Feb 6, 2019 •

edited

Loading

mortenwh commented Feb 24, 2019 •

edited

Loading