Add progress bar support for fetching and drawing HiPS tiles

[adl1995]

As mentioned in #80, I have added a progress bar support for fetching and drawing HiPS tiles. The progress bar is off by default, the result can be seen in this notebook: https://github.com/adl1995/HIPS-to-Py/blob/master/examples/Precise%20drawing-ICRS.ipynb

To achieve this, I had to make use of the tqdm module.

[adl1995]

@cdeil The test fails with this error:

ModuleNotFoundError: No module named 'tqdm'

Do we have to manually specify the required packages on Travis CI?

[cdeil]

Do we have to manually specify the required packages on Travis CI?

Yes, you have to add a new dependency like tqdm to .travis.yml as well as to setup.py. A similar example is mypy:

.travis.yml
77:               PIP_DEPENDENCIES='mypy pycodestyle'

setup.py
114:        'mypy>=0.501',

One question I have is whether we should make this a required or optional dependency!?

I think I would prefer if it's an optional dependency, but that would mean that the implementation has to be changed to remove the import tqdm at the top of the file and instead only import it when the user wants it:

def get_progress_bar

and then use it like this:

if self.progress_bar:
    from tqmd import tqmd
    tiles = tqmd(self.draw_tiles, desc='Drawing tiles')
else:
    tiles = self.draw_tiles

# Then the same as before
for tile in tiles:
    # process the tile

---

I'm also worried a bit about adding complex stuff like progress bars without any tests that show that it works. But I think the complexity of testing the progress bar functionality is probably unreasonably high that we don't attempt this, but instead just test it ourselves and rely on users to report issues if there are any now or in the future as the code changes.

[adl1995]

@cdeil I have made tqdm an optional dependency now, please check. I verified it on this notebook: https://github.com/adl1995/HIPS-to-Py/blob/master/examples/Precise%20drawing-ICRS.ipynb

[coveralls]

Coverage decreased (-0.6%) to 96.118% when pulling e2c9c69 on adl1995:progress_bar into a90e5fe on hipspy:master.

[adl1995]

@cdeil I made the changes. The tests are passing locally with the progress bar turned on. Let's see if Travis CI build passes.

[cdeil]

@adl1995 - See the fails on travis-ci. Can you fix or should I explain how?

[adl1995]

Okay, the test fails because it can't find the tqdm module. As this is an optional dependency, should I still update the .travis.yml file?

[cdeil]

As this is an optional dependency, should I still update the .travis.yml file?

I only had a very quick look, but yes, I think you need updates to .travis.yml and also to mark the tests as requires_dependency('tqdm') or to change the tests so that they run with progress_bar=False, no?

[adl1995]

I added tqdm as a dependency, however, I haven't marked the tests with requires_dependency('tqdm') as I don't know where to import this marker from.

[coveralls]

Coverage decreased (-0.3%) to 96.429% when pulling fcb26cf on adl1995:progress_bar into a90e5fe on hipspy:master.

[cdeil]

I would remove the "4.15 or later" here. Probably any tqdm version for years back will work with the basic functionality we use.

[cdeil]

Again: I would suggest to remove the version requirement on tqdm here, because what we use is just simple functionality that probably has been there for a long time.

[cdeil]

Thanks!

[cdeil]

Small follow-up commit in master: 3d06423