maximvelichko / pyvera

A python library to control devices via the Vera hub
GNU General Public License v2.0
26 stars 31 forks source link

Update __init__.py #151

Closed sresam89 closed 3 years ago

sresam89 commented 3 years ago

Fixing the "Problem parsing pin code string too many values to unpack (expected 4) "

sresam89 commented 3 years ago

@pavoni or @vangorra could you please approve the pull request

pavoni commented 3 years ago

Just for clarity - you also need to fix the build error.

Looks like your imports are not sorted correctly.

sresam89 commented 3 years ago

Made all the changes requested (except for passing the deviceIsntance to lock) and pushed the code, let me know your comments

Just for clarity - you also need to fix the build error.

Looks like your imports are not sorted correctly.

Could you help me with this Am running this on a windows machine. Not able to use your build scripts

pavoni commented 3 years ago

What’s currently failing is black. Even if you can’t run the build script, you should be able to run the individual utilities.

You can see what’s being run here: https://github.com/pavoni/pyvera/blob/master/scripts/build.sh

If you can’t track down the issues I’ll check out your branch and run them, but I won’t be able to do that until mid next week.

pavoni commented 3 years ago

You can read the build results here (although the black error isn’t very helpful!)

https://github.com/pavoni/pyvera/pull/151/checks?check_run_id=3615579670

sresam89 commented 3 years ago

What’s currently failing is black. Even if you can’t run the build script, you should be able to run the individual utilities.

You can see what’s being run here: https://github.com/pavoni/pyvera/blob/master/scripts/build.sh

If you can’t track down the issues I’ll check out your branch and run them, but I won’t be able to do that until mid next week.

Fixed black error on my local machine Not sure why its breaking on the build-process @pavoni

image

pavoni commented 3 years ago

You’ve still got black errors on your changes to __init__.py

sresam89 commented 3 years ago

You’ve still got black errors on your changes to __init__.py

Not on my local machine @pavoni see the screenshot image

Commit Id 0a69a9648af178d0e2d6e5a7b333bbee5eeffd97 checksout clean with black on my local machine

pavoni commented 3 years ago

Don’t know if this helps you get stuff running. https://dev.to/bowmanjd/getting-started-with-python-poetry-3ica

Perhaps double check which black version you have.

Otherwise I’ll run it locally next week and let you know the problem.

Also worth making sure flake8, pylint etc run cleanly.

sresam89 commented 3 years ago

Don’t know if this helps you get stuff running. https://dev.to/bowmanjd/getting-started-with-python-poetry-3ica

Perhaps double check which black version you have.

Otherwise I’ll run it locally next week and let you know the problem.

Also worth making sure flake8, pylint etc run cleanly.

My python version python -V Python 3.6.9

pip list `

astroid (2.3.3)
atomicwrites (1.4.0)
attrs (20.3.0)
backports.entry-points-selectable (1.1.0)
biopython (1.79)
CacheControl (0.12.6)
cachy (0.3.0)
certifi (2020.6.20)
cffi (1.14.6)
chardet (3.0.4)
charset-normalizer (2.0.5)
cleo (0.8.1)
clikit (0.6.2)
coverage (4.5.4)
crashtest (0.3.1)
cryptography (3.4.8)
cycler (0.10.0)
dataclasses (0.8)
distlib (0.3.2)
entrypoints (0.3)
filelock (3.0.12)
flake8 (3.7.8)
html5lib (1.1)
idna (2.10)
importlib-metadata (2.0.0)
importlib-resources (5.2.2)
isort (4.3.21)
jeepney (0.7.1)
keyring (21.8.0)
kiwisolver (1.3.1)
lazy-object-proxy (1.4.3)
lockfile (0.12.2)
matplotlib (3.3.4)
mccabe (0.6.1)
mock (1.0.1)
more-itertools (8.6.0)
msgpack (1.0.2)
mypy (0.740)
mypy-extensions (0.4.3)
numpy (1.19.5)
packaging (20.4)
pastel (0.2.1)
pathspec (0.9.0)
pexpect (4.8.0)
Pillow (8.3.2)
pip (9.0.1)
pkg-resources (0.0.0)
pkginfo (1.7.1)
platformdirs (2.3.0)
pluggy (0.13.1)
poetry (1.1.8)
poetry-core (1.0.4)
ptyprocess (0.7.0)
py (1.9.0)
pycodestyle (2.5.0)
pycparser (2.20)
pydocstyle (4.0.1)
pyflakes (2.1.1)
pylev (1.4.0)
pylint (2.4.3)
pyparsing (2.4.7)
pysam (0.16.0.1)
pytest (5.2.1)
pytest-cov (2.8.1)
python-dateutil (2.8.2)
pyvera (0.3.14)
regex (2021.8.28)
requests (2.24.0)
requests-toolbelt (0.9.1)
responses (0.10.6)
RUST (0.1.1)
SecretStorage (3.3.1)
semantic-version (2.8.5)
setuptools (58.0.4)
setuptools-rust (0.12.1)
shellingham (1.4.0)
six (1.15.0)
snowballstemmer (2.0.0)
toml (0.10.0)
tomli (1.2.1)
tomlkit (0.7.2)
typed-ast (1.4.1)
typing-extensions (3.7.4.3)
urllib3 (1.25.11)
virtualenv (20.7.2)
wcwidth (0.2.5)
webencodings (0.5.1)
wheel (0.37.0)
wrapt (1.11.2)
zipp (1.2.0)

`

i got hold a linux pc and ran the build script, and now lint complaint about the following, which does not makes sense to me. Strangely the same error goes away if include the isinstance check you asked me to remove image

sresam89 commented 3 years ago

Adding a test commit to verify possible Lint Issues https://github.com/python/mypy/issues/8763

pavoni commented 3 years ago

The build is still failing saying 4 files would be reformatted.

I’ll run locally and post the results in the next few days if you’re unable to reproduce with the ‘build’ script locally.

pavoni commented 3 years ago

You can see the failing build here!

https://github.com/pavoni/pyvera/pull/151/checks?check_run_id=3646361399

pavoni commented 3 years ago

This is the change black made for me:-

diff --git a/pyvera/__init__.py b/pyvera/__init__.py
index dffc8bc..4e0a4f6 100755
--- a/pyvera/__init__.py
+++ b/pyvera/__init__.py
@@ -566,20 +566,13 @@ class VeraDevice:
         )

     def set_door_code_values(
-        self,
-        service_id: Union[str, Tuple[str, ...]],
-        operation: str,
-        parameter: dict,
+        self, service_id: Union[str, Tuple[str, ...]], operation: str, parameter: dict
     ) -> requests.Response:
         """Add or remove door code on the vera Lock.

         This will call the Vera api to change Lock code.
         """
-        payload = {
-            "id": "lu_action",
-            "action": operation,
-            "serviceId": service_id,
-        }
+        payload = {"id": "lu_action", "action": operation, "serviceId": service_id}
         for param in parameter:
             payload[param] = parameter[param]
         result = self.vera_request(**payload)
(END)
sresam89 commented 3 years ago

diff --git a/pyvera/init.py b/pyvera/init.py

Which version of black are you using?

pavoni commented 3 years ago

‘poetry’ should be taking care of that. You can see ‘19.3b0’ in the lock file.

if you’re comfortable - I’ll just try and commit an update of ‘init’ that works to your branch.

pavoni commented 3 years ago

Black still wants you to remove the final ,

diff --git a/pyvera/__init__.py b/pyvera/__init__.py
index 648dbff..4e0a4f6 100755
--- a/pyvera/__init__.py
+++ b/pyvera/__init__.py
@@ -566,7 +566,7 @@ class VeraDevice:
         )

     def set_door_code_values(
-        self, service_id: Union[str, Tuple[str, ...]], operation: str, parameter: dict,
+        self, service_id: Union[str, Tuple[str, ...]], operation: str, parameter: dict
     ) -> requests.Response:
         """Add or remove door code on the vera Lock.

(END)
pavoni commented 3 years ago

Apart from the final black change I think this is now GTG.

Happy to merge and release when the build is clean.

sresam89 commented 3 years ago

‘poetry’ should be taking care of that. You can see ‘19.3b0’ in the lock file.

if you’re comfortable - I’ll just try and commit an update of ‘init’ that works to your branch.

poetry is not doing that with respect to black (I had to install it manually)

(.venv) sreeram@ubuntu-sam:~/hassio/pyvera$ poetry install
Installing dependencies from lock file                                 

No dependencies to install or update

Installing the current project: pyvera (0.3.14)
(.venv) sreeram@ubuntu-sam:~/hassio/pyvera$ pip list | grep b*
DEPRECATION: The default format will switch to columns in the future. You can use --format=(legacy|columns) (or define a format=(legacy|columns) in your pip.conf under the [list] section) to disable this warning.

(.venv) sreeram@ubuntu-sam:~/hassio/pyvera$ which black
/home/sreeram/.local/bin/black
(.venv) sreeram@ubuntu-sam:~/hassio/pyvera$ black --version
black, version 19.10b0

I uninstalled black at this point to show you

(.venv) sreeram@ubuntu-sam:~/hassio/pyvera$ poetry install
Installing dependencies from lock file

No dependencies to install or update

Installing the current project: pyvera (0.3.14)
(.venv) sreeram@ubuntu-sam:~/hassio/pyvera$ black --version
bash: /home/sreeram/.local/bin/black: No such file or directory
(.venv) sreeram@ubuntu-sam:~/hassio/pyvera$ 
sresam89 commented 3 years ago

How long does this take to update on https://pypi.org/project/pyvera/ ??

pavoni commented 3 years ago

I need to do a release - which I’ll do later today.

The update is then almost instant.

pavoni commented 3 years ago

Now released as 0.3.14.

Thanks for the contribution.

sresam89 commented 3 years ago

I know we had a conversation on this already. Is it worth re-visiting the "pincode" being dict versus list ? I totally understand your view on maintaining the stable code base, just trying an attempt to make it better.

pavoni commented 3 years ago

For items which are indexed only by an integer - the best data structure is really an array rather than a dict.

But unless there are a lot of pins - and a decent amount of looking up going on I think the advantages of changing to another data structure are small - and offset by the fact that we might break existing code using the API.

The library is't being actively developed - so I think t's wise to keep non critical changes to a minimum.