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

Enum #109

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Enum #109

wants to merge 14 commits into from

Conversation

scls19fr
Copy link
Contributor

Try to fix #70

I don't have most boards so some tests are skipped maybe some tests are failing.

@Vido
Copy link
Contributor

Vido commented Jan 24, 2016

@scls19fr ,

Thanks for your contribution.

I have some remarks, and they will be pointed on the diff lines.

install: pip install -qq flake8 pytest pytest-cov tox
install:
- pip install -qq flake8 pytest pytest-cov tox
- python setup.py install
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this is necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's necessary to install dependencies (enum34). So as enum34 is in dependencies of this package, installing package will install dependencies

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, because you added enum34 to setup.py and to requirements.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moreover it's a way to ensure that this package installs correctly

@Vido
Copy link
Contributor

Vido commented Feb 19, 2016

@scls19fr,

Ok, I will test (py2 and py3) your PR, if everything is ok, it might be merged.

I'm not sure if imports like this will work on Python2.7:

--- from grove import GrovePi
+++ from .grove import GrovePi

I'v done some tests:

ValueError: Attempted relative import in non-package

@Vido
Copy link
Contributor

Vido commented Feb 20, 2016

@scls19fr,

A question, does you patch supports python3?

I have found 3 problems:

  • Some tests break in Python3
        self._pin_states = pingo.util.StrKeyDict()
        # All pins start on LOW
        # FIXME: use "LOW" instead of 0
>       for location, pin in iteritems(self.pins):
            self._pin_states[location] = 0 if hasattr(pin, 'state') else None

pingo/ghost/ghost.py:53:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = ItemsView({'7': <DigitalPin @7>, '11': <PwmPin @11>, 'A1': <AnalogPin @A1>, '0': <DigitalPin @0>, '4': <DigitalPin @4>...mPin @9>, 'VCC': <VccPin 5.0V>, '12': <DigitalPin @12>, 'GND': <GroundPin>, '10': <PwmPin @10>, 'A4': <AnalogPin @A4>})

    def __iter__(self):
>       for key in self._mapping:
            yield (key, self._mapping[key])
E           TypeError: iter() returned non-iterator of type 'KeysView'
  • There are many files were pingo.LOW or pingo.OUT should be replaced. Specially on exemples/
  • requirements.txt fails on Python3, because wsgiref
Collecting wsgiref==0.1.2 (from -r requirements.txt (line 16))
  Using cached wsgiref-0.1.2.zip
    Traceback (most recent call last):
      File "<string>", line 20, in <module>
      File "/tmp/pip-build-7zil_kkb/wsgiref/setup.py", line 5, in <module>
        import ez_setup
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):

      File "<string>", line 20, in <module>

      File "/tmp/pip-build-7zil_kkb/wsgiref/setup.py", line 5, in <module>

PS: I don't think enum lead to a more elegant API... but ok.

# Blink

import time
import pingo

# Now another import is needed...
+++from pingo import Mode

board = pingo.detect.get_board()
led = board.pins[13]
---led.mode = pingo.OUT
+++led.mode = Mode.OUT

while True:
    led.toggle()
    print(led.state)
    time.sleep(.5)

@scls19fr
Copy link
Contributor Author

I don't think that there is Python 3 support for now see #100
So I'm testing with Python 2.7
I think we should first tackle Enum enhancement and after Python 2/3 support.
We can avoid to import Mode and State by exposing all possible modes and states in __init__.py but I haven't done it now to find more easily where code is broken.
Anyway 1 or 2 additional import is not so problematic. On the other side it adds completion which is very useful, especially for beginners.

@scls19fr
Copy link
Contributor Author

Any news about this ?

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c6bb567 on scls19fr:enum into * on pingo-io:master*.

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.

Reimplement constant groups as Enums
5 participants