nasa / pyCMR

Client for CMR APIs
Apache License 2.0
23 stars 12 forks source link

pyCMR Overhaul #17

Closed lewismc closed 7 years ago

lewismc commented 7 years ago

Hi Folks, This PR essentially overhauls pyCMR setup.py and codebase to build and run on Python 3.X. Additionally, it relocates the test directory to a new location hence cleaning up the directory structure. Tests are currently failing due to #16

$ nosetests
...
======================================================================
ERROR: test suite for <class 'pyCMR.tests.test_cmr_integration.TestCMRIntegration'>
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/site-packages/nose/suite.py", line 210, in run
    self.setUp()
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/site-packages/nose/suite.py", line 293, in setUp
    self.setupContext(ancestor)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/site-packages/nose/suite.py", line 316, in setupContext
    try_run(context, names)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/site-packages/nose/util.py", line 471, in try_run
    return func()
  File "/usr/local/cmr/pyCMR/tests/test_cmr_integration.py", line 18, in setUpClass
    cls.cmr = CMR('cmr.cfg')
  File "/usr/local/cmr/pyCMR/pyCMR.py", line 65, in __init__
    raise IOError("The config file can't be opened for reading/writing")
OSError: The config file can't be opened for reading/writing

----------------------------------------------------------------------
Ran 0 tests in 0.043s

FAILED (errors=1)
lewismc commented 7 years ago

Please let me know how we can address #16 then I will get the tests working @manilmaskey thank you.

lewismc commented 7 years ago

@manilmaskey @mileswwatkins @lindsleycj @scisco @amarouane @ghrcdaac CC you here for context... I would appreciate any review.

mileswwatkins commented 7 years ago

@lewismc, I like this in general, and great job tuning it up! As long as you're going in that direction and cleaning up the codebase, could you add basic flake8 linting to the tests?

Also, I believe that there are NASA folk who're using this in production, so the moving of the libraries (to more standard locations) may break their workflow; worth talking to @amarouane or @manilmaskey to assure that those people get a heads-up.

mileswwatkins commented 7 years ago

Also, @lewismc, there's another NASA-sponsored Python CMR client library in town, which might be worth checking out if you can't wait for this to get reviewed or merged in: https://github.com/jddeal/python-cmr

lewismc commented 7 years ago

Hi @mileswwatkins thanks for the heads up. IMHO we should merge these efforts and transition the unified toolkit over to the NASA Github Org. The Python toolkit you point to currently provides a pretty intuitive set of classes for query functionality only, where as this toolkit offers slightly more. A merge should not be difficult by any means.

lewismc commented 7 years ago

Now that @amarouane added an cmr.cfg.example file, if you copy this file to cmr.cfg and run tests you get the following which is much better. I think we should create a dummy user account and have a test-only cmr.cfg.example file available to pass to the CMR Class in TestCMRIntegration

lmcgibbn@LMC-056430 /usr/local/cmr(master) $ nosetests
nose.config: INFO: Ignoring files matching ['^\\.', '^_', '^setup\\.py$']
nose.selector: INFO: /usr/local/cmr/README.md is executable; skipped
nose.selector: INFO: /usr/local/cmr/cmr.cfg is executable; skipped
nose.selector: INFO: /usr/local/cmr/cmr.cfg.example is executable; skipped
nose.selector: INFO: /usr/local/cmr/pyCMR/read_eol_sf.py is executable; skipped
nose.selector: INFO: /usr/local/cmr/pyCMR/read_variable_nc.py is executable; skipped
Since these are order-sensitive integration tests, ... ERROR
Make sure that the correct number of items are returned by searches ... ok

======================================================================
ERROR: Since these are order-sensitive integration tests,
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/site-packages/nose/case.py", line 134, in run
    self.runTest(result)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/site-packages/nose/case.py", line 152, in runTest
    test(result)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/unittest/case.py", line 648, in __call__
    return self.run(*args, **kwds)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/unittest/case.py", line 608, in run
    self._feedErrorsToResult(result, outcome.errors)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/unittest/case.py", line 538, in _feedErrorsToResult
    result.addError(test, exc_info)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/site-packages/nose/proxy.py", line 132, in addError
    self.result.addError(self.test, self._prepareErr(err))
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/site-packages/nose/result.py", line 61, in addError
    exc_info = self._exc_info_to_string(err, test)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/site-packages/nose/result.py", line 187, in _exc_info_to_string
    return _TextTestResult._exc_info_to_string(self, err, test)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/unittest/result.py", line 186, in _exc_info_to_string
    exctype, value, tb, limit=length, capture_locals=self.tb_locals)
  File "/Users/lmcgibbn/miniconda3/lib/python3.5/traceback.py", line 481, in __init__
    self.filename = exc_value.filename
AttributeError: 'ParseError' object has no attribute 'filename'
-------------------- >> begin captured logging << --------------------
root: INFO: ======== Waiting for response ========
requests.packages.urllib3.connectionpool: INFO: Starting new HTTPS connection (1): cmr.uat.earthdata.nasa.gov
requests.packages.urllib3.connectionpool: DEBUG: "GET /search/collections?page_size=50&page_num=1 HTTP/1.1" 200 30421
requests.packages.urllib3.connectionpool: INFO: Starting new HTTPS connection (1): cmr.uat.earthdata.nasa.gov
requests.packages.urllib3.connectionpool: DEBUG: "GET /search/collections?page_size=50&page_num=2 HTTP/1.1" 200 8811
root: INFO: ======== Waiting for response ========
requests.packages.urllib3.connectionpool: INFO: Starting new HTTPS connection (1): cmr.uat.earthdata.nasa.gov
requests.packages.urllib3.connectionpool: DEBUG: "GET /search/granules?page_size=50&page_num=1 HTTP/1.1" 200 15643
requests.packages.urllib3.connectionpool: INFO: Starting new HTTPS connection (1): cmr.uat.earthdata.nasa.gov
requests.packages.urllib3.connectionpool: DEBUG: "GET /search/granules?page_size=50&page_num=2 HTTP/1.1" 200 15576
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------
Ran 2 tests in 21.845s

FAILED (errors=1)
mileswwatkins commented 7 years ago

IMHO we should merge these efforts and transition the unified toolkit over to the NASA Github Org

I'd agree that that's the ideal plan, and that combining the code wouldn't be too bad, but I think the catch is that both libraries already have uses in production, and so altering the API of either, much less merging them and moving to a new address, has a nontrivial political cost. If you can swing it, power to ya ;D

lewismc commented 7 years ago

@mileswwatkins yes it's always the same at NASA ;) My general feeling is that both toolkits are half baked and they would benefit, and so would the communities which use them, if we could merge the efforts under a unified project. There's no reason we couldn't drum up support for this on one of the DAAC managers telecon's.

Once we have things sorted out over here with #17 I'll ping whoever the maintainer is of the other library with the concept. Thanks again for the insight... it's a shame that these packages haven't been more widely advertised at ESDIS events such as the recent ESDSWG meeting in Annapolis.

mileswwatkins commented 7 years ago

Sure thing, and totally agree that it could be a big win to publicize a unified Python tool; CMR's such a powerful tool, but can have a learning curve to understand and use natively.

FWIW, I've also created a NodeJS CMR client library (also used in production) with the same CMR capabilities as this Python library. It's not currently public because we haven't cleared export control, but it will be public (and FOSS) later this year. I'm sure that releasing this alongside a Python tool could compound the success.

cc @ianschuler, @matthewhanson

lewismc commented 7 years ago

ack

lewismc commented 7 years ago

hi folks, is anyone able to review this pull request?

lewismc commented 7 years ago

Thanks @amarouane I'm going to start work on the community building over at the other CMR Python toolkit. I wonder if you have any suggestions for stabilizing the build on this project?