Skip to content

Fix validator bug where list_of_list conversion didn't capture inner list type#308

Merged
hdunham merged 4 commits into
developfrom
validator_list_of_list
Mar 25, 2022
Merged

Fix validator bug where list_of_list conversion didn't capture inner list type#308
hdunham merged 4 commits into
developfrom
validator_list_of_list

Conversation

@hdunham
Copy link
Copy Markdown
Collaborator

@hdunham hdunham commented Mar 8, 2022

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?

(Bug fix, feature, docs update, ...)
bug fix

What is the current behavior?

(You can also link to an open issue here)
When a 2D list was provided for coincident_peak_load_active_timesteps, and it had float values in it that were invalid time steps (e.g 1.2), the validator would not return an error. In the JuMP model, the floats would be used as indices and result in a REoptFailedToStart error for the user. When a 2D list provided had float values but they were integers in float form (e.g. 1.0), the validator would not convert these to integers, resulting also in a REoptFailedToStart error.

What is the new behavior (if this is a feature change)?

Any time float values are provided in coincident_peak_load_active_timesteps, the validator converts them to integers if possible otherwise gives the input error "Could not convert coincident_peak_load_active_timesteps () in Scenario>Site>ElectricTariff to one of int,list_of_int,list_of_list"

Does this PR introduce a breaking change?

(What changes might users need to make in their application due to this PR?)
no

Other information:

hdunham added 4 commits March 8, 2022 15:16
there was no bug caused by it being a float field, I'm just changing for correctness
validator was trying to convert to list of list again right after it failed, but without considering the inner list type
Comment thread reo/models.py
chp_does_not_reduce_demand_charges = models.BooleanField(null=True, blank=True)
emissions_region = models.TextField(null=True, blank=True)
coincident_peak_load_active_timesteps = ArrayField(ArrayField(models.FloatField(null=True, blank=True), null=True, default=list), null=True, default=list)
coincident_peak_load_active_timesteps = ArrayField(ArrayField(models.IntegerField(null=True, blank=True), null=True, default=list), null=True, default=list)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this change require a migration?

Copy link
Copy Markdown
Member

@NLaws NLaws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I just have the question about needing a database migration for the type change in models.py. Please test deployment on testranch.

@hdunham hdunham merged commit f576be3 into develop Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants