Feature/import sector benchmarks #59

tsubik · 2019-09-17T09:24:56Z

Changing the structure of CP::Benchmarks

Previously CP::Benchmark has benchmarks jsonb column with different scenarios and emissions values for them. I decided to flatten that structure to incorportate scenario in the model and emissions jsonb. Import/export and data management for single benchmark is easier now, it also makes more sense.

Import/export of CP Benchmarks on the new admin page for benchmark management.

Edit form for CP Benchmark with dynamic emissions table

To reimport existing benchmarks

bundle exec rake import:cp_benchmarks

kowal · 2019-09-17T10:22:20Z

When adding new emission values like this:

.. no error is raised and the value is not added. I think we should either show an error or allow to update it without the emission.

tsubik · 2019-09-17T10:54:24Z

Hmm, I think it worked without providing emission, let me check

tsubik · 2019-09-17T11:07:15Z

Ok, @kowal I fixed that.

kowal · 2019-09-17T14:35:11Z

app/admin/cp_benchmarks.rb

+  end
+
+  csv do
+    year_columns = collection.flat_map(&:emissions_all_years).uniq.sort


It might be more readable to extract this somewhere. Maybe some decorator / helper?

Hmm, just this one line? not sure if that would help for me this is readable

kowal · 2019-09-17T14:36:49Z

app/admin/sectors.rb

-                table_for benchmark.benchmarks, class: 'cell-padding-sm cell-centered' do
-                  column :name do |b|
-                    b['name']
+            resource.cp_benchmarks.latest_first.group_by(&:release_date).map do |release_date, benchmarks|


here as well, some logic could be extracted to view helpers

kowal · 2019-09-17T14:38:47Z

app/models/cp/benchmark.rb

+    end
+
+    def average_emission
+      not_empty_values = emissions.values.compact


none_empty_values?

kowal · 2019-09-17T14:41:45Z

app/services/csv_import/cp_benchmarks.rb

+    def parse_float(value)
+      return unless value.present?
+
+      value.to_f


value&.to_f but maybe current code is more readable..

I will change it back, actually, it does not make sense to keep empty values. The same I will change in the interface, empty values will be discarded when saving.

kowal · 2019-09-17T14:45:28Z

app/services/csv_import/cp_benchmarks.rb

+
+    def emissions(row)
+      row.headers.grep(/\d{4}/).map do |year|
+        {year.to_s.to_i => parse_float(row[year])}


row.headers.grep(EMISSION_VALUE_PATTERN) .map { |year| {year.to_i => parse_float(row[year])} } # to_s is really needed? .reduce(&:merge)

Also I got a feeling we can replace .map {}.reduce { } with just one .reduce { }.

true, good point

not sure why to_s is here, maybe it was needed. I copied that from climate watch, but I can remove it, we will see :P

ok, I know why to_s year is a symbol.

kowal · 2019-09-18T06:47:57Z

app/models/cp/benchmark.rb


-    validates_presence_of :date
+    validates_presence_of :release_date, :scenario


Maybe we should validate scenario with inclusion on the known scenario list (https://github.com/Vizzuality/laws_and_pathways/pull/57/files#diff-bcd1663b967ed112eee68c615f3ac627R4)?

@simaob do you know if that list (of scenarios) should be open or rather strict? Do we need to manage this list as well?

I would keep that as just a name with the flexibility to change to whatever. They already have different names for different release dates and sectors. As long we don't need any complex reports it should be fine.

hmm but later we will need only a known list of scenario on the frontend. How will we map it if there are more scenario types in the DB?

You only need names, and those benchmarks need to be grouped by release_date and only one benchmark group should be taken (everything depends on assessment date - an appropriate benchmark should be taken by date). Scenario names will be from that chosen benchmark group.

Not sure if the colour has any meaning, I can try to double check with Fausto. If not we can have a number of colours that are flexible enough to an expected number of scenarios and then sort them always the same way, so that they always get the same colour. Something like that?

We can only show data against one benchmark set/group.

If the color has meaning from the lowest to the highest scenario we can add colors based on the average of emission values, scenario name won't matter that much.

oh right, good point!

@kowal is right here, on the image above I can only see the top of the chart, but looks like that chart needs standardized scenario names across different sectors. They are not standardized now, so I believe in the next iteration when we know that all scenarios are the same we can add validation to be only those from strict list.

tsubik added 13 commits September 16, 2019 15:32

add CPBenchmarks uploader

16c538c

add export CP benchmarks

3cae0fc

download related cp benchmarks

e9e71ab

cp benchmark import add id

6a72de7

cp benchmarks import specs

e1f86ae

flatten cp::benchmarks

d03ab65

fix cp benchmarks import

bb2ffe9

adding cp benchmarks admin pages

12a103c

use normal AA csv export

bd5eebf

cp benchmark form fix styling

9675867

fix cp benchmarks json edit

71a7b88

fix import issues, sort by average emission reverse

70de9e2

Merge branch 'develop' into feature/import-sector-benchmarks

4685fcf

tsubik requested review from kowal and simaob September 17, 2019 09:24

fix benchmarks csv test file

e621854

cp benchmark emissions allow to save values without value

db8343f

cp benchmark emission parseFloat

13eadcf

kowal reviewed Sep 17, 2019

View reviewed changes

kowal mentioned this pull request Sep 18, 2019

(feature): TPI (sectors, companies) controllers, chart API & views #57

Merged

14 tasks

kowal reviewed Sep 18, 2019

View reviewed changes

kowal requested a review from agwozdowski September 18, 2019 10:23

little refactoring

45788b2

tsubik force-pushed the feature/import-sector-benchmarks branch from f1fdd08 to 45788b2 Compare September 18, 2019 12:31

Merge branch 'develop' into feature/import-sector-benchmarks

dc8e8ad

kowal approved these changes Sep 18, 2019

View reviewed changes

tsubik merged commit 51d20cb into develop Sep 18, 2019

tsubik deleted the feature/import-sector-benchmarks branch September 18, 2019 12:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/import sector benchmarks #59

Feature/import sector benchmarks #59

tsubik commented Sep 17, 2019

kowal commented Sep 17, 2019

tsubik commented Sep 17, 2019

tsubik commented Sep 17, 2019

kowal Sep 17, 2019

tsubik Sep 18, 2019

kowal Sep 17, 2019

kowal Sep 17, 2019

kowal Sep 17, 2019

tsubik Sep 18, 2019

kowal Sep 17, 2019

tsubik Sep 18, 2019

tsubik Sep 18, 2019

tsubik Sep 18, 2019

kowal Sep 18, 2019

tsubik Sep 18, 2019

kowal Sep 18, 2019

tsubik Sep 18, 2019

simaob Sep 18, 2019

tsubik Sep 18, 2019 •

edited

Loading

tsubik Sep 18, 2019

simaob Sep 18, 2019

tsubik Sep 18, 2019 •

edited

Loading


		validates_presence_of :date
		validates_presence_of :release_date, :scenario

Feature/import sector benchmarks #59

Feature/import sector benchmarks #59

Conversation

tsubik commented Sep 17, 2019

kowal commented Sep 17, 2019

tsubik commented Sep 17, 2019

tsubik commented Sep 17, 2019

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

tsubik Sep 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsubik Sep 18, 2019 • edited Loading

Choose a reason for hiding this comment

tsubik Sep 18, 2019 •

edited

Loading

tsubik Sep 18, 2019 •

edited

Loading