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

merge new changes #9

Merged
merged 16 commits into from
Nov 30, 2015
Merged

merge new changes #9

merged 16 commits into from
Nov 30, 2015

Conversation

mrtommyb
Copy link
Owner

No description provided.

mrtommyb added a commit that referenced this pull request Nov 30, 2015
@mrtommyb mrtommyb merged commit 85c576f into mrtommyb:master Nov 30, 2015
@rhandberg
Copy link

There seems to have been introduced an error on line 124 of K2onSilicon.py when running in Python 2.
Instead of

    info = CAMPAIGN_DICT["c{}".format(fieldnum)]

you could use

    info = CAMPAIGN_DICT["c%d" % (fieldnum)]

which works fine.

@mrtommyb
Copy link
Owner Author

mrtommyb commented Dec 1, 2015

The code runs on python 2 for me although I get the following warning

/Users/tom/.virtualenvs/p1/lib/python2.7/site-packages/matplotlib/collections.py:590: FutureWarning: elementwise comparison failed; returning scalar instead, but in the future will perform elementwise comparison
  if self._edgecolors == str('face'):

@mrtommyb
Copy link
Owner Author

mrtommyb commented Dec 1, 2015

regarding @rhandberg comment,

 info = CAMPAIGN_DICT["c{}".format(fieldnum)]

looks like legal python to me.
as a simple example the following returns True Python 2.7.9

d = {'1': 'success'}
d['{}'.format(1)] == 'success'

@bsipocz
Copy link

bsipocz commented Dec 1, 2015

String .format may cause problems for older pythons (I could reproduce it on 2.6).

One easy fix for the code to be more explicit:

info = CAMPAIGN_DICT["c{0}".format(fieldnum)]

Other option for @rhandberg is to upgrade python to have a newer 2.7

@barentsen
Copy link
Contributor

@rhandberg I have fixed this problem in v2.0.1 which is available over at https://github.com/KeplerGO/K2fov. I have also added 2.6 to Travis unit testing so it shouldn't break again.

Please consider upgrading to Python 3 when you can.

@rhandberg
Copy link

@barentsen Sounds great! We will properly have to update in the not-so-distant future, but it will be to 2.7 and not 3 :-)

@barentsen
Copy link
Contributor

@rhandberg Just a thought to keep in mind: all the standard packages (numpy, matplotlib, astropy, etc) have been fully compatible with Python 3 for years and have started talking about dropping Python 2.7 support, so that they can finally take advantage of the nice new features that have been added to Python 3 in the past 7 (!) years of Python core development (e.g. the asyncio module, which is a killer feature for scientific computing).

I fully understand that there are often practical reasons not to switch, and I'm sure K2 tools will keep supporting Python 2, but I enjoy spreading the word that Python 3 is ready and now is a great time to switch! =)

mrtommyb added a commit that referenced this pull request Dec 9, 2015
mrtommyb added a commit that referenced this pull request Dec 9, 2015
mrtommyb added a commit that referenced this pull request Dec 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants