Skip to content

Update variable names #70

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

Merged
merged 14 commits into from
Jul 5, 2015
Merged

Update variable names #70

merged 14 commits into from
Jul 5, 2015

Conversation

wholmgren
Copy link
Member

Lots of API changes going into this PR. Not ready yet, just getting it going early since it relates to #69.

to do:

  • snlinverter
  • sapm outputs
  • calcparams_desoto kwargs and doc string
  • absoluteairmass arg
  • relativeairmass arg
  • ineichen output
  • haurwitz arg and output
  • lots in irradiance.py

@wholmgren wholmgren added the api label Jun 22, 2015
@wholmgren wholmgren added this to the 0.2 milestone Jun 22, 2015
@wholmgren
Copy link
Member Author

I changed some variable names that we hadn't yet discussed, so commenters in #37 may want to look at the changes to singlediode.

@wholmgren
Copy link
Member Author

I'm sure I missed some things, but this now goes most of the way to closing #37. This is a good time for comments/requests.

I'll see about updating the tutorials tomorrow. @NelisW updated the tutorials in his fork, and I want to look at that too. Sure would be nice if GitHub could show better notebook diffs.

The TravisCI fails are all due to #72. Everything works on 2.7.

@@ -195,7 +195,9 @@ def ineichen(time, location, linke_turbidity=None,

cos_zenith = tools.cosd(ApparentZenith)

clearsky_GHI = cg1 * I0 * cos_zenith * np.exp(-cg2*AMabsolute*(fh1 + fh2*(TL - 1))) * np.exp(0.01*AMabsolute**1.8)
clearsky_GHI = ( cg1 * I0 * cos_zenith *
Copy link
Contributor

Choose a reason for hiding this comment

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

would also change clearsky_GHI to all lower case (clearsky_ghi) for consistency.
Not sure, did you intended to change only names for the kw arguments (all api related) or all internal variable names too?

Copy link
Member Author

Choose a reason for hiding this comment

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

I only intended to change the API related names for now. I would like to see the internal variables get updated, but I think we can do this in smaller chunks.

@NelisW
Copy link

NelisW commented Jun 25, 2015

Trying my best to get the notebooks working. Two remaining (pvsystem.ipynb, Test_Scripts_1.ipynb) that need some attention. The refactoring broke some of the notebooks at quite a deep level. Hope to spend some time again tonight. In this part of the world we have semi-regular load shedding - was down again last night for 2 hours. You can work on a laptop, but have no internet connection - makes you realise how dependent we have become on the internet.

@wholmgren
Copy link
Member Author

@NelisW I made some updates to the pvsystem notebook in this PR since I wanted to plot some things to make sure that it was all ok. Test_Scripts_1 and TS_to_irradiance haven't been touched in a long, long time.

@wholmgren wholmgren force-pushed the variablenames branch 4 times, most recently from 9b27585 to ee8e3b7 Compare June 26, 2015 01:57
@wholmgren
Copy link
Member Author

The tests now pass by forcing the use of numpy 1.8 and scipy 0.14 on Python 3. You can install any of the pandas versions on top if this if you use --no-deps. I will update the readme separately.

@wholmgren
Copy link
Member Author

@bmu do you think we need to update all of the internal variables in this PR or can we deal with that later?

@bmu
Copy link
Contributor

bmu commented Jun 29, 2015

No Problem to deal with it later.

Am 29. Juni 2015 17:50:54 MESZ, schrieb Will Holmgren notifications@github.com:

@bmu do you think we need to update all of the internal variables in
this PR or can we deal with that later?


Reply to this email directly or view it on GitHub:
#70 (comment)

@wholmgren
Copy link
Member Author

@bmu @Calama-Consulting I reviewed this again and I think it's ready to merge if you're happy with the changes.

bmu added a commit that referenced this pull request Jul 5, 2015
@bmu bmu merged commit d8987e5 into pvlib:master Jul 5, 2015
@wholmgren wholmgren deleted the variablenames branch October 28, 2015 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants