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

Fix tests, imports and readme #64

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

frankharkins
Copy link

@frankharkins frankharkins commented Apr 12, 2021

Change 1 (fix imports)

On trying to import:

from melopy.scales import generateScale

I got this error:

In [1]: from melopy.scales import generateScale
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
<ipython-input-1-8766c174d199> in <module>
----> 1 from melopy.scales import generateScale

~/.local/lib/python3.8/site-packages/melopy/scales.py in <module>
      5 class MelopyValueError(ValueError): pass
      6 
----> 7 from utility import note_to_key, key_to_note
      8 
      9 def bReturn(output, Type):

ModuleNotFoundError: No module named 'utility'

Which was fixed by adding 'melopy.' to the start of each melopy import.

Change 2 (fix tests)

The tests were still failing, I think this is because key_to_note was expecting integer division with /. I have changed this to // and the tests pass (and the results make sense). Additionally, had to move tests/melopy_tests.py to top level (called it tests.py).

Change 3 (fix readme)

Fixes #63

Change 4

Added __future__ statements to make compatible with Python 2.

Thanks.

@frankharkins frankharkins changed the title Fix tests and imports Fix tests, imports and readme Apr 12, 2021
@asteriskman7
Copy link
Collaborator

I think what you are seeing is due to the fact that this was originally designed to work with python 2, not python 3. I don't think that it would be wise to make changes that break python 2 support.

@frankharkins
Copy link
Author

frankharkins commented Apr 13, 2021

Python 2 was sunset a while ago, although I think a simple __future__ import might let this work with both. Let me try tonight.

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.

Readme major_scale example is wrong
2 participants