Skip to content

Improvement xls decoding#1

Open
RamParameswaran wants to merge 2 commits intojericmac:masterfrom
RamParameswaran:Improvement--xls_decoding
Open

Improvement xls decoding#1
RamParameswaran wants to merge 2 commits intojericmac:masterfrom
RamParameswaran:Improvement--xls_decoding

Conversation

@RamParameswaran
Copy link
Copy Markdown

This PR applies two patches to fix failing tests:

  1. testGetSecurityInfoLive --> previously used ticker symbol ("CBAPC") has been suspended on the ASX. Replaced with "CBAPD"
    Original error:
ERROR: testGetSecurityInfoLive (pyasx.tests.data.securities.SecuritiesTest)
Unit test for pyasx.data.securities.get_security_info()
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ram/Dropbox/My Projects/pyasx/pyasx/tests/data/securities.py", line 210, in testGetSecurityInfoLive
    security = pyasx.data.securities.get_security_info('CBAPC')
  File "/home/ram/Dropbox/My Projects/pyasx/pyasx/data/securities.py", line 166, in get_security_info
    "Unknown security ticker %s" % ticker
pyasx.data.UnknownTickerException: Unknown security ticker CBAPC
  1. testGetListedSecuritiesLive --> error decoding the XLS file for listed securities. In this PR, I'm using Pandas to read the excel file. This simplifies the code. But it adds two dependencies: pandas and xlrd
    Original error:
ERROR: testGetListedSecuritiesLive (pyasx.tests.data.securities.SecuritiesTest)
Unit test for pyasx.data.securities.get_listed_securities()
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ram/Dropbox/My Projects/pyasx/pyasx/tests/data/securities.py", line 139, in testGetListedSecuritiesLive
    securities = pyasx.data.securities.get_listed_securities()
  File "/home/ram/Dropbox/My Projects/pyasx/pyasx/data/securities.py", line 48, in get_listed_securities
    temp_stream.write(block.decode('unicode_escape'))
UnicodeDecodeError: 'unicodeescape' codec can't decode bytes in position 809-810: malformed \N character escape

Great library 👍

- Fix test implementation details
- Mock pandas.read_excel function
@ShahinNamin
Copy link
Copy Markdown

get_listed_securities throws an error, please investigate
The error is thrown on line 48 of securities.py and is
"UnicodeDecodeError: 'unicodeescape' codec can't decode bytes in position 809-810: malformed \N character escape"

@RamParameswaran
Copy link
Copy Markdown
Author

@ShahinNamin that's the original error which I addressed in point number 2 in my PR comment (above).

I re-ran the tests on my fork to be certain. I get all tests passing. Can you please confirm if there's a discrepancy?

/home/ram/Desktop/pyasx-Improvement-xls_decoding/pyasx_test/lib/python3.6/site-packages/requests/__init__.py:80: RequestsDependencyWarning: urllib3 (1.24.1) or chardet (3.0.4) doesn't match a supported version!
  RequestsDependencyWarning)
testGetCompanyAnnouncementsLive (pyasx.tests.data.companies.CompaniesTest) ... ok
testGetCompanyAnnouncementsMocked (pyasx.tests.data.companies.CompaniesTest) ... ok
testGetCompanyInfoLive (pyasx.tests.data.companies.CompaniesTest) ... ok
testGetCompanyInfoMocked (pyasx.tests.data.companies.CompaniesTest) ... ok
testGetListedCompaniesLive (pyasx.tests.data.companies.CompaniesTest) ... ok
testGetListedCompaniesMocked (pyasx.tests.data.companies.CompaniesTest) ... ok
testGetListedSecuritiesLive (pyasx.tests.data.securities.SecuritiesTest) ... ok
testGetListedSecuritiesMocked (pyasx.tests.data.securities.SecuritiesTest) ... ok
testGetSecurityInfoLive (pyasx.tests.data.securities.SecuritiesTest) ... ok
testGetSecurityInfoMocked (pyasx.tests.data.securities.SecuritiesTest) ... ok

----------------------------------------------------------------------
Ran 10 tests in 4.116s

OK

@ShahinNamin
Copy link
Copy Markdown

My bad, the error is in the master branch. If your pull request is approved, it will be resolved. Thanks @RamParameswaran

@RichardScottOZ
Copy link
Copy Markdown

Thanks for this.

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.

3 participants