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

Fix sim.extra regex to match Python 2.7.x where x is two digits #394

Merged
merged 2 commits into from Jun 6, 2017

Conversation

MBradbury
Copy link
Contributor

Fix sim.extra regex to match Python 2.7.x versions where x is two digits. Also support Python 3 and development versions of Python.

I posted this fix to the tinyos.users mailing list, but the post hasn't been approved yet. See the thread at: https://www.millennium.berkeley.edu/pipermail/tinyos-help/2017-January/059055.html

…its. Also support Python 3 and development versions of Python.
Copy link
Member

@cire831 cire831 left a comment

Choose a reason for hiding this comment

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

looks like the previous version would only match against Python 2.x something.

the new version looks like it will match any Python x.x.x that probably isn't what is intended.

I'm pretty sure it needs to be something Python 2.x...

@MBradbury
Copy link
Contributor Author

Actually it was intended. On my fork I added support for Python 3 and thought it would be easier to catch any digit, rather than just 2 or 3.

@cire831
Copy link
Member

cire831 commented Jan 22, 2017

does it break anything?

@MBradbury
Copy link
Contributor Author

It doesn't seem to do so. Simulations give different results with the same seed, but they're still deterministic. Had to tweak a few files to replace xrange with range and also modify the swig bindings.

However, I've ended up going back to 2.7 due to slower performance.

In terms of the regex, it should just be more general and haven't seen it break. I've also used it with things like 3.6-dev and pyenv installed versions of python.

@cire831
Copy link
Member

cire831 commented Jan 23, 2017

okay. I'm okay with going ahead with this. However....

I'm not okay with accepting any version of python which I beleive the patch will do.

I'd suggest you change it to recognize any version of 2.7 (one or two digits) and any version of 3

sound reasonable?

@MBradbury
Copy link
Contributor Author

Sorry for the long gap, this got lost.

I've pushed a fix so that only Python 2 or Python 3 is matched.

@cire831 cire831 merged commit 9447577 into tinyos:master Jun 6, 2017
@cire831
Copy link
Member

cire831 commented Jun 6, 2017

no worries

thanks for making the effort

@MBradbury MBradbury deleted the py-version-fix branch June 7, 2017 10:08
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

2 participants