Fix/eliminate warnings #990
Conversation
The test seems to be designed to test if warnings can be turned off. However, the operation never created one.
* pytest.approx instead of roundin * series.iloc[int] instead of series[int]
The ExperimentalFeatureWarning is only issued in multi-period-mode, which is experimental anyway. I don't think we need two warnings.
DataFrame.groupby with axis=1 is deprecated. The message says to do `frame.T.groupby(...)` without axis instead. So did I.
This reverts commit 1a681f0.
This reverts commit 8ef4c4c.
|
The test just fails because Zenodo is down. I think, it's ready for review. Comment on the two reverted commits: First, I decided to suppress warnings about experimental features, as they don't add value: We test them and do not need to be warned they are used unintentionally. However, there were issues with that, as not all tests were run because of this. So, I decided to keep the warnings. |
|
I'd say everything looks fine here, thank you! One note on using |
By default, |
|
Decreased coverage is decreased because I deleted lines. I hope this is in order. |
|
Thank you, @p-snft! I must admit that I overlooked this PR as I was quite busy with other developments. I'll add my review shortly. |
There was a problem hiding this comment.
Thank you very much, @p-snft and sorry for me taking quite some time for the review. I'm happy to approve. Also, I resolved some conflicts that were introduced by my last feature and detected and resolved one bug in the course of doing so that was neither detected by the test nor by me (please see my comment on this one).
| timeindex=timeindex, | ||
| timeincrement=[1] * len(timeindex), | ||
| periods=[timeindex], | ||
| infer_last_interval=False, |
There was a problem hiding this comment.
I see that you where looking to explicitly set infer_last_interval.
I noticed that this test was somewhat outdated since I used the old periods definition (type dict) here which I fixed in #977 which I just merged.
Another thing is that though it was syntactically correct with the list attribute as in the current dev branch, it would not have worked to set up a model this way. Since the test is only to demonstrate a warning that occurs before the model setup, I'm sorry that have not detected this. Anyways, as it is now, also the model build would work.
Concerning the (not so nice) inconsistency to be resolved some day, see #979.
There are a lot of FutureWarnings partly from imported packages but also due to our own prepared API changes. This PR is meant to adapt to these (planned) changes.