pr-omethe-us / PyKED

Python interface to the ChemKED database format
https://pr-omethe-us.github.io/PyKED/
BSD 3-Clause "New" or "Revised" License
15 stars 15 forks source link

Import the __version__ attribute to the top-level module #99

Closed bryanwweber closed 6 years ago

bryanwweber commented 6 years ago

Changes proposed in this pull request:

from pyked import __version__
print(__version__)

@pr-omethe-us/chemked

kyleniemeyer commented 6 years ago

@bryanwweber this looks fine to me, but the AppVeyor build failed for some reason...

bryanwweber commented 6 years ago

Looks like it failed because one of the expected ResourceWarnings didn't show up. We might need to loosen the criteria on that test... or perhaps figure out a way to fix Habanero so it doesn't do that (I think its Habanero, anyways). I restarted the build, let's see what happens

bryanwweber commented 6 years ago

Well, I don't know why the tests are failing, but it's not actually a problem with the code itself, its a problem with the test. Do you want to merge, or do you want me to try to fix it?

kyleniemeyer commented 6 years ago

I wonder if this is another package version issue?

kyleniemeyer commented 6 years ago

We probably need to try and fix this, because I don't think we'll have an updated Windows package until we do.

kyleniemeyer commented 6 years ago

Welp, I don't think it's a habanero version issue, because it looks like 0.6.0 in both the Windows and Linux test logs.

kyleniemeyer commented 6 years ago

I wonder if it's an issue with pytest, not habanero...

Here's the first failing test:

____________________ TestGetReference.test_valid_reference ____________________
self = <pyked.tests.test_converters.TestGetReference object at 0x000000604E661B38>
    def test_valid_reference(self):
        """Ensure valid reference reads properly.
            """
        root = etree.Element('experiment')
        ref = etree.SubElement(root, 'bibliographyLink')

        ref.set('doi', '10.1016/j.ijhydene.2007.04.008')
        ref.set('preferredKey', 'Chaumeix, N., Pichon, S., Lafosse, F., Paillard, C.-E., '
                'International Journal of Hydrogen Energy, 2007, (32) 2216-2226, '
                'Fig. 12., right, open diamond'
                )

        with pytest.warns(UserWarning) as w:
            ref = get_reference(root)
>       assert w[1].message.args[0] == ('Using DOI to obtain reference information, '
                                        'rather than preferredKey.'
                                        )
pyked\tests\test_converters.py:145: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = WarningsChecker(record=True), i = 1
    def __getitem__(self, i):
        """Get a recorded warning by index."""
>       return self._list[i]
E       IndexError: list index out of range
C:\Miniconda36-x64\envs\py3.5\lib\site-packages\_pytest\recwarn.py:139: IndexError

That last bit is kinda weird, no?

bryanwweber commented 6 years ago

Alright, I'll try to dig into this over the holiday.

codecov[bot] commented 6 years ago

Codecov Report

Merging #99 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #99   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         879    880    +1     
  Branches      212    212           
=====================================
+ Hits          879    880    +1
Impacted Files Coverage Δ
pyked/tests/__init__.py 100% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6b214c0...4ee30e4. Read the comment docs.

bryanwweber commented 6 years ago

The issue is with Habanero, and more specifically, that they use the Requests library which leads to a bunch of ResourceWarnings (see https://github.com/requests/requests/issues/1882). Apparently (based on these results), how all that gets handled is different on Windows vs. *nix. Anyhow, I've updated how we handle the warnings in the test suite so it should hopefully be cross platform now.

bryanwweber commented 6 years ago

All passed, LGTM.