Skip to content
This repository has been archived by the owner on Apr 13, 2021. It is now read-only.

I181 glo decoder python binding #340

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

valeri-atamaniouk
Copy link
Contributor

Python bindings for GLONASS message decoder

To be merged after #337

@coveralls
Copy link

Coverage Status

Coverage increased (+1.5%) to 56.651% when pulling baba16f on valeri-atamaniouk:i181-glo-decoder-python-binding into d269099 on swift-nav:master.

@valeri-atamaniouk valeri-atamaniouk force-pushed the i181-glo-decoder-python-binding branch from baba16f to c9a1f43 Compare May 6, 2016 09:58
@coveralls
Copy link

Coverage Status

Coverage remained the same at 56.701% when pulling c9a1f43 on valeri-atamaniouk:i181-glo-decoder-python-binding into 5ae607f on swift-nav:master.

@valeri-atamaniouk
Copy link
Contributor Author

@gsmcmullin Please review. All dependencies are now satisfied.

maintainer = 'Swift Navigation',
maintainer_email = '[email protected]',
packages = ['swiftnav'],
name='swiftnav',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there extra spaces here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The formatting has changed to to auto-formatting (autopep.8 --indent-size 2). Shall it be 2 here and below? Do we have any other non-default settings (other than indentation size)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I will ask... @benjamin0 Do you know what the story is with our Python formatting?

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@valeri-atamaniouk looks like it should be two spaced ident

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ljbade @benjamin0 https://www.python.org/dev/peps/pep-0008/#id3
https://www.python.org/dev/peps/pep-0008/#fn-hi
-Hanging indents should add a level

The issue is related to PEP8 conformance of E121 and E126. I suggest to update the coding style to mention that hanging indents are 2 spaces. It would make it impossible to use auto-indentation, but I'm perfectly fine with that.

For example, when using pep8 to check the style with only 2 spaces in hanging indents:

$ pep8 --ignore E111,E114 setup.py
setup.py:5:3: E121 continuation line under-indented for hanging indent
setup.py:55:7: E121 continuation line under-indented for hanging indent
setup.py:63:5: E121 continuation line under-indented for hanging indent

Choose a reason for hiding this comment

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

I think if hanging indents should add a level and we define a level as indented two spaces, then hanging indents are an additional 2 spaces. @valeri-atamaniouk feel free to update the linked hackpad above. @fnoble do you have an opinion?

@valeri-atamaniouk valeri-atamaniouk force-pushed the i181-glo-decoder-python-binding branch from c9a1f43 to b58bf98 Compare May 23, 2016 08:53
@coveralls
Copy link

Coverage Status

Coverage remained the same at 59.112% when pulling b58bf98 on valeri-atamaniouk:i181-glo-decoder-python-binding into 8bbc35e on swift-nav:master.

Added Python bindings for GLONASS message decoder operations.
@valeri-atamaniouk valeri-atamaniouk force-pushed the i181-glo-decoder-python-binding branch from b58bf98 to 6e3c44a Compare May 24, 2016 11:09
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 59.157% when pulling 6e3c44a on valeri-atamaniouk:i181-glo-decoder-python-binding into 1f08046 on swift-nav:master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants