Skip to content

add the dataclasses for object metadata and conftest#14

Merged
alorenzo175 merged 7 commits into
SolarArbiter:masterfrom
alorenzo175:datafixtures
Feb 27, 2019
Merged

add the dataclasses for object metadata and conftest#14
alorenzo175 merged 7 commits into
SolarArbiter:masterfrom
alorenzo175:datafixtures

Conversation

@alorenzo175

@alorenzo175 alorenzo175 commented Feb 21, 2019

Copy link
Copy Markdown
Contributor

with examples of what metadata, observations, and forecasts will look like

closes #12

@alorenzo175

Copy link
Copy Markdown
Contributor Author

@lboeman @wholmgren what do you think about dataclasses? They are nice because they can be "frozen" and be accessed easily. They will require python 3.7 though.

Comment thread solarforecastarbiter/datamodel.py Outdated
Comment thread solarforecastarbiter/datamodel.py Outdated

@wholmgren wholmgren left a comment

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.

Based only on an hour of research into dataclasses, I'd say that they seem useful for defining the containers in the datamodel. Alternatives are standard classes, dicts, namedtuples. Aside from version compatibility, I don't see an advantage to a standard class or namedtuple for this application. dicts are nice because they are so simple, but perhaps too flexible.

requiring 3.7 is pretty aggressive, but I'm not opposed.

Comment thread solarforecastarbiter/datamodel.py Outdated
Comment thread solarforecastarbiter/datamodel.py Outdated
Comment thread solarforecastarbiter/datamodel.py Outdated
@alorenzo175

Copy link
Copy Markdown
Contributor Author

@cwhanse @dplarson @awig Do any of you have reactions to using dataclasses for the site, forecast, and observation metadata? They provide a nice (almost) immutable container but Python 3.7 is required. I want to make sure 3.7 doesn't hinder development (although a conda create -n sfacore python=3.7 should get an environment going easily).

@cwhanse

cwhanse commented Feb 22, 2019

Copy link
Copy Markdown
Contributor

We have had issues testing pvlib in py3.7, although off the top of my head, I can't recall what they are. Something related to numpy functions.

@wholmgren

Copy link
Copy Markdown
Member

@cwhanse I think pvlib/pvlib-python#634 solved those issues. I've been using 3.7 in some conda environments for the last few months without issues.

@cwhanse

cwhanse commented Feb 22, 2019

Copy link
Copy Markdown
Contributor

That was my reaction. For me objections is None from a brief scan of the py 3.7.2 release note. I'm not currently using py 3.7.

@alorenzo175

Copy link
Copy Markdown
Contributor Author

@wholmgren Seems like 3.7 is ok. Availability and curtailment were added. anything else?

@wholmgren

Copy link
Copy Markdown
Member

merge/rebase on master so your test runs

@alorenzo175 alorenzo175 merged commit 41d74a1 into SolarArbiter:master Feb 27, 2019
@alorenzo175 alorenzo175 deleted the datafixtures branch February 27, 2019 21:10
@wholmgren wholmgren added this to the 1.0 alpha milestone Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

make pytest data fixtures for metrics and validation

4 participants