-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Replace freq '1M' with '1MS' #2266
base: main
Are you sure you want to change the base?
Conversation
I ended up removing the Note, test failures are unrelated. |
pvlib/iotools/sodapro.py
Outdated
# For time_steps '1d' and '1MS' drop timezone and round to nearest midnight | ||
if (time_step == '1d') | time_step.endswith('MS'): # noqa | ||
data.index = pd.DatetimeIndex(data.index.date) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kandersolar Is it desirable that the index becomes a date instead of datetime for frequencies of day and month?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the distinction? Does pandas have some kind of DateIndex
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No they do not. The operation converts the datetimeindex to just an index (where each element is a datetime.date object.
data.index = pd.DatetimeIndex(data.index.date)
This is however inconvenient as you can't use the convenient pandas datetime operations such as resample etc.
docs/sphinx/source/whatsnew
for all changes. Includes link to the GitHub Issue with:issue:`num`
or this Pull Request with:pull:`num`
. Includes contributor name and/or GitHub username (link with:ghuser:`user`
).remote-data
) and Milestone are assigned to the Pull Request and linked Issue.The
freq='1M'
is used in a couple of places in the pvlib iotools functions, which results in the following future warning:The frequency 'M' is being phased out in favor of 'ME' and 'MS' which correspond to month end and month start. 'M' is equivalent to 'ME'. However, 'MS' is the only one supported in our minimum Pandas version, whereas 'ME' was first introduced in a later version. For the iotools functions, I think 'MS' is conceptually more correct.
Minimum changes to the lookup linked turbidity functions were made to make tests pass. I preferred making these modifications over calculating new test values.