Skip to content

Documentation improvement #335 #336

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 17 commits into from
Jul 3, 2017
Merged

Documentation improvement #335 #336

merged 17 commits into from
Jul 3, 2017

Conversation

birgits
Copy link
Contributor

@birgits birgits commented Jun 8, 2017

This PR implements the docstring issues addressed in #335.

@birgits
Copy link
Contributor Author

birgits commented Jun 8, 2017

I checked all other occurrences of surface_tilt and think the docstring only needs to be changed in the PVSystem class.
I will also add the default values to the docstrings in the next days.

@wholmgren wholmgren added this to the 0.4.6 milestone Jun 8, 2017
@birgits
Copy link
Contributor Author

birgits commented Jun 14, 2017

I added the default values in modelchain.py. I looked how default values are added in other libraries such as numpy, pandas, etc. but there doesn't seem to be one common way. I chose the numpy way but can change it if you like. When we agree on how we want the default values to appear in the docstring, I can also add them to the other modules.

@birgits
Copy link
Contributor Author

birgits commented Jun 14, 2017

I could also change the orientation_strategy default (#290) if you like. Should this be done in a new PR?

@wholmgren
Copy link
Member

I don't have any preference on the style. Maybe others can chime in if they care. There are a couple of pvlib functions that already have defaults, and I think they're in the pandas style.

#290 should be done in a separate PR.

@birgits
Copy link
Contributor Author

birgits commented Jun 22, 2017

I added all default values in pandas style. Just let me know if there is anything you don't agree with.

@wholmgren
Copy link
Member

Thanks, @birgits, this is great. Can you add a note and your name to the 0.4.6 whatsnew file?

@wholmgren
Copy link
Member

The failing tests are not related to this PR, so I will go ahead and merge.

@wholmgren wholmgren merged commit eef1992 into pvlib:master Jul 3, 2017
@wholmgren wholmgren modified the milestones: 0.4.6, 0.5.0 Aug 8, 2017
@wholmgren wholmgren modified the milestones: 0.5.0, 0.4.6 Aug 8, 2017
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.

2 participants