-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
Updating to package template 1.1.1 #177
Conversation
version = 'dev' | ||
|
||
packagename = os.path.basename(os.path.dirname(__file__)) | ||
TESTED_VERSIONS[packagename] = version |
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.
This line seems to be causing a bunch of test failures.
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.
This is due to unrelated conda issues (yet another one, these past couple of days are quite messed up), simply a too old astropy is picked up.
So I would wait with this until those things in astropy/ci-helpers#98 are ironed out.
@bmorris3 - Can you restart this travis, please? |
@bsipocz: Things that are still failing
|
Ouch, OK, I'll put in on the todo. On 21 June 2016 at 17:00, Brett M. Morris notifications@github.com wrote:
|
e6c1b7c
to
35217a2
Compare
OK, this is now rebased, hopefully it will solve the issues. |
@bmorris3 - The fail seems to be related to a new conda issue that just popped up now that the previous ones are solved. I'm on it, but have to admit it's not easy to get fixed while trying to focus on dotastro instead ;) |
Skipping doctests that aren't useful, causing warnings on py2.7
skipping is_night doctest
@bmorris3 - I'm not sure what to do with the remaining two failures. The first one seems totally out of context (is that download triggered somewhere else, but there is a delay with the output?). For the second one I see the warning locally right before the test header is printed out, but still got the failure later. I guess the warnings are set to be raised only once, thus the test can't catch it? |
I'm not sure what that's all about. I'm hoping to chat with @eteq to track down what the new IERS behavior is in astropy, which may be the culprit. |
Hmm, the first of the second (:confused:) error is not exactly due to the IERS machinery - it's because when the tests are run in a clean environment they have to download the latest IERS. That's fine except that the first time that happens it prints the message about the download. and that's confusing pytest. So we just have to convince pytest not to worry about that (probably by having a fixture that runs at the start of all the tests that just makes sure the latest IERS are in place.) The |
Oh, but I think the first error is a travis issue - I've seen that happen sometimes just because travis is messed up temporarily. I restarted it just to see if it resolves itself now. |
I was testing what |
The first error occurs every time with |
I think allowing the remote tests to fail would be the best option unless we can specifically allow the doctest failure that is caused in |
Indeed @kvyh! Thanks. I had two open questions regarding the fix, but can merge it asap, too and move the discussion here instead. |
fixed _low_precision to throw the warning when off the table
Something is getting rather awful here. With the fix of @kvyh the second error goes away, but the first error gets even worse when building against the dev version. To start with the behaviour is different locally, with 1.2 it finds IERS_A in the cache, while with dev it goes into the else part
The failure with astropy dev:
|
… the cache or download the fresh version
So it's still not clear why there is a behaviour difference between 1.2 and dev astropy, but adding AttributeError to the exceptions and thus raising a warning is I think an acceptable workaround. Also using iers.IERS_Auto.open() rather than iers.IERS.open() solves the problem of downloading a newer version of the table right at the beginning, so no more errors for the doctests. However I would appreciate if someone more familiar with the IERS infrastructure would give the last few commits it a thorough review. |
This is weird, the remote tests now pass locally for py3, but not for py2 while both still fail on travis. Do you want me to rebase and remove the rather experimental last few commits that clearly didn't work as intended? |
4f7f0b9
to
d45a636
Compare
@bmorris3 @eteq - This is readyto go. There are a few workarounds included, but those I think we should live with them until they are fixed either upstream (e.g. astropy/astropy#5194), or independently of this PR in astroplan. |
# Workaround until astropy/astropy#5194 is solved | ||
try: | ||
iers_a = IERS_A.open(download_file(IERS_A_URL, cache=True)) | ||
except HTTPError: |
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.
@taldcroft - I though about using IERS_Auto.open() here, but the sake of this example is to show the difference between A and B, etc, and if A is not available this becomes a bit pointless. However with fresh eyes and brain my solution seems even more hacky than yesterday, and inclined to use the astropy machinery.
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.
@bsipocz - I think the intent is that eventually astroplan will switch to the astropy
machinery, right @bmorris3 ? So then there's no need for this at all. Basically this IERS stuff was a stand-in until the machinery in astropy got working.
So with that in mind it doesn't really matter that much one way or another I guess. ;)
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.
OK, good to know. I'll try to come back to this in the weekend to clean up.
Sorry to pester, but if this is passing can we get it merged? Myself and others want to do some work with astroplan but it currently won't install from source or from pip because of the |
@bsipocz Thanks for letting me know. @bmorris3 @eteq Any ideas when this can happen? This is causing all of our travis-ci builds to fail and is very annoying. If it's not going to happen within a day or two then the |
Updating to package template 1.1.1
No description provided.