Skip to content
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

Split Offshore and Onshore Wind for Reeds (#27) #28

Merged
merged 6 commits into from
Jan 30, 2020
Merged

Conversation

MRossol
Copy link
Collaborator

@MRossol MRossol commented Jan 23, 2020

Implementation of #27:

  • Add offshore kwarg to ReedsClassifier which will pull out offshore (offshore=True) or onshore (offshore=False) or ignore (offshore=None)
  • Add a split_offshore value to the config, if True it will run the CLI twice, once with offshore=True, and once with offshore=False

@MRossol MRossol added the feature New feature or request label Jan 23, 2020
@MRossol MRossol self-assigned this Jan 23, 2020
@MRossol
Copy link
Collaborator Author

MRossol commented Jan 30, 2020

@grantbuster do you mind reviewing this before I merge?

Copy link
Member

@grantbuster grantbuster left a comment

Choose a reason for hiding this comment

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

Some advice on a json loads issue that I ran into with the reV CLI's. Formatting dict strings for CLI's is so difficult... I've tried json.dumps() before but click had issues with the format. I left a comment with the best method I found.

cluster_kwargs=kwargs)
table_full, table, agg_table = out
cluster_kwargs=kwargs,
filter=json.loads(filter))
Copy link
Member

Choose a reason for hiding this comment

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

FYI you might need to do something like i did in the link below. Basically if there is a stringified dict "{col: False}" you need to replace "False" with "false" and "None" with "null" for json loads to work properly.

https://github.com/NREL/reV/blob/38e19cf5278a425d239c0dd8f0cc34f25c87e45d/reV/supply_curve/cli_aggregation.py#L195-L200

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Converted this into a utility function here and added it to the reeds_cli
NREL/reV#108

"""
rev_table = self._parse_table(rev_table)
if filter is not None:
Copy link
Member

Choose a reason for hiding this comment

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

Great

@grantbuster grantbuster merged commit f48a3e0 into master Jan 30, 2020
@grantbuster grantbuster deleted the reeds_offshore branch January 30, 2020 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants