-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Local datasets scaling graph v2 #77
Changes from all commits
510f0a3
ebf1bb2
58d68a5
bdf08b4
199d2b1
6f47a35
82a4d1e
310ab25
62cffb5
8f85716
fdc9e3f
a3ea7ce
065345b
9e6e6e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,15 @@ | ||
module Atlas | ||
class GraphPersistor | ||
def initialize(dataset, path) | ||
@dataset = dataset | ||
@path = path | ||
end | ||
|
||
def self.call(dataset, path) | ||
new(dataset, path).persist! | ||
end | ||
|
||
def persist! | ||
File.open(@path, 'w') do |f| | ||
f.write EssentialExporter.dump(refinery_graph).to_yaml | ||
end | ||
end | ||
|
||
private | ||
|
||
def refinery_graph | ||
Runner.new(@dataset).refinery_graph(:export) | ||
end | ||
# Public: Builds the graph and exports it to a YAML file. | ||
# | ||
# dataset - This dataset's graph will be built and persisted | ||
# path - File to which the graph will be exported | ||
# export_modifier - Will be called on the graph's exported hash prior to saving it | ||
# | ||
# Returns a Hash | ||
GraphPersistor = lambda do |dataset, path, export_modifier: nil| | ||
data = EssentialExporter.dump(Runner.new(dataset).refinery_graph(:export)) | ||
export_modifier.call(data) if export_modifier | ||
File.write(path, data.to_yaml) | ||
data | ||
end | ||
end |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,17 +8,31 @@ def initialize(base_dataset_key, derived_dataset_name, number_of_residences) | |
|
||
def create_scaled_dataset | ||
derived_dataset = Dataset::DerivedDataset.new( | ||
@base_dataset.attributes | ||
.merge(scaled_attributes) | ||
.merge(new_attributes)) | ||
@base_dataset.attributes. | ||
merge(AreaAttributesScaler.call(@base_dataset, scaling_factor)). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpicking: can this be indented 2 lines further for the sake of readability? I don't care to strongly about it though so I'm fine either way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in commit 8f85716 |
||
merge(new_attributes)) | ||
|
||
derived_dataset.save! | ||
|
||
GraphPersistor.call(@base_dataset, derived_dataset.graph_path) | ||
GraphPersistor.call(@base_dataset, derived_dataset.graph_path, export_modifier: Scaler::GraphScaler.new(scaling_factor)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The thing is that I would like to avoid the word "scaling" completely inside graph_persistor.rb since the latter has nothing to do with the former. |
||
|
||
TimeCurveScaler.call(@base_dataset, scaling_factor, derived_dataset) | ||
end | ||
|
||
private | ||
|
||
def value | ||
@number_of_residences | ||
end | ||
|
||
def base_value | ||
@base_dataset.number_of_residences | ||
end | ||
|
||
def scaling_factor | ||
value.to_r / base_value.to_r | ||
end | ||
|
||
def new_attributes | ||
id = Dataset.all.map(&:id).max + 1 | ||
{ | ||
|
@@ -28,20 +42,13 @@ def new_attributes | |
key: @derived_dataset_name, | ||
area: @derived_dataset_name, | ||
base_dataset: @base_dataset.area, | ||
scaling: scaling, | ||
scaling: | ||
{ | ||
value: value, | ||
base_value: base_value, | ||
area_attribute: 'number_of_residences', | ||
}, | ||
} | ||
end | ||
|
||
def scaling | ||
{ | ||
value: @number_of_residences, | ||
base_value: @base_dataset.number_of_residences, | ||
area_attribute: 'number_of_residences', | ||
} | ||
end | ||
|
||
def scaled_attributes | ||
ScaledAttributes.new(@base_dataset, @number_of_residences).scale | ||
end | ||
end | ||
end | ||
end # Scaler | ||
end # Atlas |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
module Atlas | ||
class Scaler::AreaAttributesScaler | ||
# Only attributes common to FullDataset and DerivedDataset | ||
# may be scaled | ||
SCALEABLE_AREA_ATTRS = Atlas::Dataset.attribute_set | ||
.select { |attr| attr.options[:proportional] }.map(&:name).freeze | ||
|
||
def self.call(*args) | ||
new(*args).scale | ||
end | ||
|
||
def initialize(base_dataset, scaling_factor) | ||
@base_dataset = base_dataset | ||
@scaling_factor = scaling_factor | ||
end | ||
|
||
def scale | ||
result = {} | ||
SCALEABLE_AREA_ATTRS.map do |attr| | ||
if value = @base_dataset[attr] | ||
result[attr] = Util::round_computation_errors(value * @scaling_factor) | ||
end | ||
end | ||
result | ||
end | ||
end # Scaler::AreaAttributesScaler | ||
end # Atlas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current use of this method (in
TimeCurveScaler
) looks like: CSVDocument.create → add data → save.That involves two writes to disk (in
create
and again insave
), and if something goes wrong in "add data" or the second save, you end up with a half-complete CSV file on disk. Perhapscreate
could yield itself prior to writing, so that the user can add their initial data? (likeFile.open(path, 'w') { |f| ... }
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are so right 😔
I did it this way because Ruby's CSV class is not really good for read-write access and because I did not want to mess up the current
CSVDocument.new
signature.But I found an okish way to build only the document and not save it until later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be commit 310ab25