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

Changed initial logging, and added a functional example #33

Merged
merged 8 commits into from
Jun 7, 2016
Merged

Changed initial logging, and added a functional example #33

merged 8 commits into from
Jun 7, 2016

Conversation

tavurth
Copy link

@tavurth tavurth commented Jun 2, 2016

Hi I fixed a logging error "No logger available for pyoanda", and changed the data grabbing example to be more useful (With comments)

@toloco
Copy link
Owner

toloco commented Jun 2, 2016

Can you check why travis says no? it looks like something it is wrong in the code.

Thanks,


log = getLogger(__name__)

logging.basicConfig(level='CRITICAL')
Copy link
Owner

Choose a reason for hiding this comment

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

Here you are defining a logger instead of getting the defined logger, we should use this approach in case of global logger doesn't exists.

@tavurth
Copy link
Author

tavurth commented Jun 2, 2016

  • Updated the example to Python 3.x

As we can see below logging.basicConfig actually initialises the logging features, which removes a warning message when using Python 2.x

logging.basicConfig([**kwargs])
Does basic configuration for the logging system by creating a StreamHandler with a default Formatter and adding it to the root logger. 

The functions debug(), info(), warning(), error() and critical() will call basicConfig() automatically if no handlers are defined for the root logger.

https://docs.python.org/2/library/logging.html#logging.basicConfig

@coveralls
Copy link

coveralls commented Jun 2, 2016

Coverage Status

Coverage increased (+0.04%) to 69.861% when pulling 7e587e9 on tavurth:develop into a4c61b3 on toloco:develop.

@coveralls
Copy link

coveralls commented Jun 3, 2016

Coverage Status

Coverage increased (+0.04%) to 69.861% when pulling a999a8b on tavurth:develop into a4c61b3 on toloco:develop.

@coveralls
Copy link

coveralls commented Jun 6, 2016

Coverage Status

Coverage increased (+0.04%) to 69.861% when pulling d728b26 on tavurth:develop into a4c61b3 on toloco:develop.

@toloco
Copy link
Owner

toloco commented Jun 6, 2016

Thanks mate, it looks much better.

Just one very little think, can you make sure it is compliant of PEP8 too? let me know if this is too annoying...

@tavurth
Copy link
Author

tavurth commented Jun 6, 2016

Hey, I don't have a PEP8 verification method. Perhaps you know of one?

@toloco
Copy link
Owner

toloco commented Jun 6, 2016

Yay!
Not a big deal you can use the pep8 package (https://pypi.python.org/pypi/pep8).

Thanks,

@coveralls
Copy link

coveralls commented Jun 6, 2016

Coverage Status

Coverage increased (+0.04%) to 69.861% when pulling 8583fbc on tavurth:develop into a4c61b3 on toloco:develop.

@toloco toloco merged commit f97a384 into toloco:develop Jun 7, 2016
@toloco
Copy link
Owner

toloco commented Jun 7, 2016

@tavurth thanks for your contribution

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

3 participants