Skip to content

Alcarney py3#61

Merged
geraintpalmer merged 8 commits into
masterfrom
alcarney-py3
Jun 20, 2016
Merged

Alcarney py3#61
geraintpalmer merged 8 commits into
masterfrom
alcarney-py3

Conversation

@geraintpalmer
Copy link
Copy Markdown
Member

Adjustments to PR #60 by @alcarney

Fixing issues around "Inf" and float("Inf"):
* In the parameters dictionary use "inf" (more readable if printing out dictionary
* Everywhere else use float("inf") (creating a Network from dictionary converts string to float)
This is done so that users can type in "Inf" into a parameters file, and also error checking can check for string when seeing "Inf". Everywhere else in the code float("inf") is used, which makes comparisons easier and works with Python 3.

alcarney and others added 3 commits June 15, 2016 20:55
So far the following changes have been made:

- Adding extra dependency, `future`, this so you can still use `xrange`
  in python 3 by adding `from past.builtins import xrange` wherever
  needed.

- Added a `from __future__ import absolute_import` so module loading
  works in both python 2 and 3

- In python 2 you could do `generator.next()`, however this has been
  removed in python 3 in favour of `next(generator)`. Thankfully this
  also works in python 2

- In python 2 if you did the following:

  ```
  if 10 < 'Inf':
    ....
  ```
  python would automatically convert the string to a float. However
  python 3 doesn't do this. So pretty much everywhere all 'Infs' have
  been converted to float('Inf').

  In the case of loading parameters from a YAML file, I had to add the
  following to `load_paramters` in `ciw/import_params.py`:

  ```
  parameters['Queue_capacities'] =
       list(map(lambda x: float("Inf") if x == "Inf" else x,
       parameters['Queue_capacities']))
  ```
  Which simply does the conversion as the parameters are read in and
  leaving anything which doesn't need converting alone.

  *NOTE:* My gut tells me that it's this change that's causing the
  `no such attribute: self.servers` error. Since the if statement in
  the `__init__` method of the `Node` class:
  ```
  if self.c < float("Int"):
    ...
  ```
  would no longer pass and I don't see anywhere else where `self.servers`
  would be assigned.

- Finally something that you REALLY need to double check is a list
  comprehension on line 136 in `import_params.py` from:

  ```
  [len(row) for row in obs for obs in params['Transition_matrices'].values()]
  ```

  to:

  ```
  [len(row) for row in [obs for obs in params['Transition_matrices'].values()][0]]
  ```

  This is due to a bug in python 2 that caused the scope from list
  comprehensions to 'leak' into it's surroundings. In python 3 this was
  fixed and I think that is why the variable `obs` is undefined if you
  try to use the original code with python 3. However I'm not sure if my
  'fix' has changed the semantics of the above expression so I'm
  highlightling it here so you can check it over.

That's what I have so far and I don't think I've forgottenany other
changes I've had to make.
Comment thread .travis.yml
language: python
python:
- 2.7
- 3.5
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could also run py3.4? No need but no harm to actively test the two latest py3s.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep I'll add it. everything still failing atm anyway (doctests i think though)

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling f8b2b5d on alcarney-py3 into 7697e26 on master.

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 20, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 780d703 on alcarney-py3 into 7697e26 on master.

@geraintpalmer
Copy link
Copy Markdown
Member Author

@alcarney random.choice has different outcomes with the same random seed in Python 3 and Python 2. Replaced with numpy.random.choice.

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 20, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling bac59a5 on alcarney-py3 into 7697e26 on master.

@geraintpalmer geraintpalmer merged commit c0c26b6 into master Jun 20, 2016
@geraintpalmer geraintpalmer deleted the alcarney-py3 branch March 13, 2017 14:31
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.

4 participants