Migrate to hypothesis.strategies.datetimes #2368
Conversation
|
Hello @himanshukgp! Thanks for updating the PR.
Comment last updated on January 19, 2018 at 08:58 Hours UTC |
| @@ -84,7 +85,11 @@ def test_fido(query): | |||
|
|
|||
|
|
|||
| @pytest.mark.remote_data | |||
| @given(time_attr(time=datetimes(timezones=[], max_year=datetime.datetime.utcnow().year, min_year=2010))) | |||
| @given(time_attr(time=datetimes( | |||
| # timezones=None, | |||
There was a problem hiding this comment.
Why is this commented out? is it needed?
There was a problem hiding this comment.
No it is not needed. I'll remove it in next commit.
|
I think the import is used in other files/locations |
|
Yes, I am working on it. |
|
I just fixed the eve test failure that occurred on my machine. Hopefully it will pass! The change you made to the EVE urls are passed into a sunpy function as part of pytest, so those should not need to be modified. |
|
@himanshukgp Thanks for this! Sorry for getting a bits handsy on it, I was just thinking of adding this for our next release (0.8.3)! |
nabobalis
added
Tests
| ), | ||
| (TimeRange('2012/7/7', '2012/7/14'), | ||
| 'http://lasp.colorado.edu/eve/data_access/evewebdata/quicklook/L0CS/SpWx/2012/20120707_EVE_L0CS_DIODES_1m.txt', | ||
| 'http://lasp.colorado.edu/eve/data_access/evewebdata/quicklook/L0CS/SpWx/2012/20120714_EVE_L0CS_DIODES_1m.txt' | ||
| 'http://lasp.colorado.edu/eve/data_access/evewebdata/quicklook/L0CS/SpWx/2012/20120714_EVE_L0CS_DIODES_1m.txt', |
There was a problem hiding this comment.
The extra commas I've added here probably need removing.
There was a problem hiding this comment.
Commas at the end of a container don't add anything. They are though a good practice for version control if you think that list, tuple, ... will grow over time, so there's not 2 lines changed, but only 1 new addition each time. (Probably not the case here in the inner tuple, but would do on the outer list).
|
So the test failure happens since the figure envs have a hard pinned hypothesis version that is before they changes the API. So those versions need updating in the environment files under |
|
hypothesis=3.42.1=py27_1 |
|
@himanshukgp Yes that would be exactly right. |
|
@nabobalis I made the required changes but made some mistakes in the process. Please guide me on how should I move forward with this PR. |
|
Mistakes? The PR looks to be ready. I'll go over it again once more. |
|
Same commits have been pushed twice here. |
|
We can clean that up when we merge it. |
|
The CI failed due to doctests. Thanks again for this @himanshukgp |
Regarding #2275
Made required changes in sunpy/net/dataretriever/tests/test_eve.py