Skip to content
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

MNT: flake8 and datetime fixes for Travis #269

Merged
merged 4 commits into from
Jul 15, 2019

Conversation

zbruick
Copy link
Contributor

@zbruick zbruick commented Jun 28, 2019

Travis had a lot of flake8 and a few assertion errors due to changes in how Pandas handles datetimes/Timestamps. I also updated the contributor's guide to be more in line with the MetPy version to encourage the usage of the Conda environment.

Note: I closed #268 and replaced with this identical PR due to a mistake I made with git in the previous branch.

…Pandas

that were breaking Travis. Also edited the contribution document to give more
information in line with the MetPy contributors' guide.
@zbruick
Copy link
Contributor Author

zbruick commented Jun 28, 2019

Not sure what the remaining Travis errors are now.

Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Thanks for running these down, just some stylistic things that I think would make these changes a bit shorter.

siphon/simplewebservice/iastate.py Outdated Show resolved Hide resolved
siphon/ncss.py Outdated Show resolved Hide resolved
siphon/tests/test_ndbc.py Outdated Show resolved Hide resolved
AUTHORS.txt Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@zbruick
Copy link
Contributor Author

zbruick commented Jul 1, 2019

All requested changes made. Added the two commits and pushed up.

@zbruick zbruick mentioned this pull request Jul 1, 2019
dopplershift
dopplershift previously approved these changes Jul 3, 2019
Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@dopplershift
Copy link
Member

Ah, but sadly timezone is not available in Python 2.7. 😭 In the http_util, there's an existing UTC class you should be able to use instead.

@zbruick
Copy link
Contributor Author

zbruick commented Jul 3, 2019

Have I said how much I can't wait till 2.7 gets laid to rest? I'll hunt this down and get it compatible with 2 and 3.

@zbruick
Copy link
Contributor Author

zbruick commented Jul 3, 2019

http.util.utc worked. Rebased and pushed.

@dopplershift
Copy link
Member

At the rate we're working on Siphon, the next release could very well be the last release we do for 2.7.

@lesserwhirls
Copy link
Collaborator

And we'll celebrate the heck out of that release, too.

Party

@zbruick
Copy link
Contributor Author

zbruick commented Jul 13, 2019

Can y'all take a look at the Travis and AppVeyor errors to see what's up? I'd like to get this cleaned up and then get #272 in early this week so that the new GLM python gallery example can be merged successfully.

@dopplershift
Copy link
Member

Trying to re-run.

@dopplershift
Copy link
Member

@zbruick One problem is that a previous work-around has us installing sphinx-gallery from git, which is now failing on Python 2.7. It looks like sphinx-gallery has been updated since, so can you try removing the line manually installing sphinx-gallery? (It's already listed in setup.py.)

@zbruick
Copy link
Contributor Author

zbruick commented Jul 13, 2019

Removed the sphinx line in setup.py and pushed.

@dopplershift
Copy link
Member

Sorry, I was unclear. The line in setup.py should stay, the line in .travis.yml doing pip install of sphinx-gallery from git needs to go.

@dopplershift
Copy link
Member

We also have a problem where the ncss/NCSS_Example.py is failing, which I can reproduce locally. It's hitting live data, so it's possible things changed out from under us. The error:

Traceback (most recent call last):
  File "NCSS_Example.py", line 66, in <module>
    ax.plot(temp[:].squeeze(), press_vals, 'r', linewidth=2)
  File "/Users/rmay/miniconda3/envs/py37/lib/python3.7/site-packages/matplotlib/axes/_axes.py", line 1666, in plot
    lines = [*self._get_lines(*args, data=data, **kwargs)]
  File "/Users/rmay/miniconda3/envs/py37/lib/python3.7/site-packages/matplotlib/axes/_base.py", line 225, in __call__
    yield from self._plot_args(this, kwargs)
  File "/Users/rmay/miniconda3/envs/py37/lib/python3.7/site-packages/matplotlib/axes/_base.py", line 391, in _plot_args
    x, y = self._xy_from_xy(x, y)
  File "/Users/rmay/miniconda3/envs/py37/lib/python3.7/site-packages/matplotlib/axes/_base.py", line 270, in _xy_from_xy
    "have shapes {} and {}".format(x.shape, y.shape))
ValueError: x and y must have same first dimension, but have shapes (34,) and (31,)

seems like we may have been doing something done before.

@zbruick
Copy link
Contributor Author

zbruick commented Jul 13, 2019

Oh that looks like a FV3 error - those dimensions bring back the memories. Should be able to implement the fix we used previously.

@zbruick
Copy link
Contributor Author

zbruick commented Jul 15, 2019

The docs are failing on the IGRA2_example with a timeout error, but the ftp link is still valid and the example runs fine on 3.7 (need to try 3.6 and 2.7 which we're actually using here). Tried rerunning Travis, but that didn't fix it.

@zbruick
Copy link
Contributor Author

zbruick commented Jul 15, 2019

Just tried 3.6 and 2.7, and was unable to recreate the timeout issue locally for that example.

@zbruick
Copy link
Contributor Author

zbruick commented Jul 15, 2019

Trying an update to Travis to allow FTP testing (see https://docs.travis-ci.com/user/common-build-problems/#ftpsmtpother-protocol-does-not-work). Will remove if the FTP connection still doesn't work.

@zbruick
Copy link
Contributor Author

zbruick commented Jul 15, 2019

So that didn't work. This data is also accessible by HTTP, and it's probably worth migrating over to that from FTP, as it will likely be dropped eventually and Travis recommends use of HTTP whenever possible (https://blog.travis-ci.com/2018-07-23-the-tale-of-ftp-at-travis-ci). I'll open an issue for this - in the meantime, what should we do?

@dopplershift
Copy link
Member

This PR didn’t cause the problem, so I’ll just merge over the failures. Rewriting IGRA to use http before the next release is the real answer.

@dopplershift dopplershift merged commit 881c77d into Unidata:master Jul 15, 2019
@dopplershift dopplershift added this to the Winter 2018 milestone Jul 18, 2019
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.

3 participants