Knowledge-Graph-Hub / neat-ml

Network Embedding All the Things
BSD 3-Clause "New" or "Revised" License
18 stars 1 forks source link

Making neat-ml poetry-fied (plus tox, black, flake8, and mypy updates) #88

Closed hrshdhgd closed 1 year ago

hrshdhgd commented 2 years ago
caufieldjh commented 2 years ago

This is pretty strange - I'm having difficulty establishing the path for neat_ml_schema. I can import it without issue, but any attempt to find its path returns None:

>>> import neat_ml_schema
>>> print(neat_ml_schema.__file__)
None
>>> import inspect
>>> inspect.getfile(neat_ml_schema)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.8/inspect.py", line 659, in getfile
    raise TypeError('{!r} is a built-in module'.format(object))
TypeError: <module 'neat_ml_schema' (namespace)> is a built-in module
>>> import pkg_resources 
>>> pkg_resources.resource_filename('neat_ml_schema','src/schema/neat_ml_schema.yaml')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/harry/neat-env/lib/python3.8/site-packages/pkg_resources/__init__.py", line 1144, in resource_filename
    return get_provider(package_or_requirement).get_resource_filename(
  File "/home/harry/neat-env/lib/python3.8/site-packages/pkg_resources/__init__.py", line 364, in get_provider
    return _find_adapter(_provider_factories, loader)(module)
  File "/home/harry/neat-env/lib/python3.8/site-packages/pkg_resources/__init__.py", line 1392, in __init__
    self.module_path = os.path.dirname(getattr(module, '__file__', ''))
  File "/usr/lib/python3.8/posixpath.py", line 152, in dirname
    p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not NoneType
hrshdhgd commented 2 years ago

Aah ... I think I know what the problem is. I need to fix it in neat-ml-schema

hrshdhgd commented 2 years ago

Alright, releasing a new version of neat-ml-schema (v0.1.11) right now. That should solve the problem. Also, the new path will be pkg_resources.resource_filename('neat_ml_schema','schema/neat_ml_schema.yaml') and this should work. I have made the change in neat-ml and will push the change once the release is done, that way gh-action must pass.

hrshdhgd commented 2 years ago

Alright, this passes! I have implemented tox which in turn implements black. There will be a lot of files committed that will be reformatted. Should we do this in this PR or a separate one?

caufieldjh commented 2 years ago

Let's take care of the reformatting here too

hrshdhgd commented 2 years ago

I will need help with flake8 error correction. Primarily because you both have a better knowledge of what each of the functions do than I. We can go through it together during our hackathons. the command to highlight the areas that need attention : poetry run tox -e flake8

caufieldjh commented 2 years ago

It's now at one of those fun points where all the tests clear on my machine but hang on the GH action

hrshdhgd commented 2 years ago

Since we are using poetry, we'll have to try to replace all pip references from unit_tests_and_lint.yml. The whole file needs a revamp. I'm not sure if that'll help but that's the way it should be in this case any ways. Similar to (or maybe exactly the same as) what we have in neat-ml-schema

hrshdhgd commented 2 years ago

Also I get 4 errors while running poetry run tox -e py linked to 2 issues:

E   ModuleNotFoundError: No module named 'tensorflow'

E   ModuleNotFoundError: No module named 'parameterized'

Are these dependencies not declared in poetry? Could the test be clearing locally because you have these dependencies installed locally outside of poetry?

hrshdhgd commented 2 years ago

When I run just pytest , my terminal hangs too without any test run.

hrshdhgd commented 2 years ago

I think qc.yml could replace unit_tests_and_lint.yml

caufieldjh commented 2 years ago

Could the test be clearing locally because you have these dependencies installed locally outside of poetry?

That's a reasonable hypothesis - I think what's happening is that the graph embedding tests I re-added are taking too long on the GH actions environment, despite the test embedding params being pretty minimal. The issue is more obvious when running poetry run python -u -m unittest.

hrshdhgd commented 2 years ago

Do we need the 2 packages (tensorflow and parameterized) mentioned above? Should we poetry add them ?

hrshdhgd commented 2 years ago

When I run poetry run python -u -m unittest it gives me 48 errors.

caufieldjh commented 2 years ago

Do we need the 2 packages (tensorflow and parameterized) mentioned above? Should we poetry add them ?

We shouldn't strictly need tensorflow but mlp_model.py does still contain import tensorflow as tf. That could cause some issues. The parameterized is a test dependency only. Wouldn't hurt to poetry add it.

hrshdhgd commented 2 years ago

When I run poetry run python -u -m unittest it gives me 48 errors.

That was my fault..correcting it.

hrshdhgd commented 2 years ago

So now I get 3 errors with the same issue

KeyError: "['ENSP00000416750', 'ENSP00000350199', 'ENSP00000398481', 'ENSP00000355589', 'ENSP00000385571', 'ENSP00000285735', 'ENSP00000451575', 'ENSP00000313600', 'ENSP00000226207', 'ENSP00000298910', 'ENSP00000357879', 'ENSP00000305260', 'ENSP00000468354', 'ENSP00000163416', 'ENSP00000314649', 'ENSP00000291906', 'ENSP00000363970'] not in index"

thoughts? To reproduce: poetry run python -u -m unittest tests/test_cli.py -k test_run_do_embeddings

Once this is fixed, we have just docstrings to add for flake8 to pass

caufieldjh commented 2 years ago

That's a good sign - I'm getting the same errors now! I think there's just a misalignment between the tests and the fixtures.

hrshdhgd commented 2 years ago

Excellent! The tests pass now ...All that is left are docstrings!!

hrshdhgd commented 2 years ago

Ok , all we need now is updating the yaml_helper.py docstrings and we should good to go! I may need help with this.

caufieldjh commented 2 years ago

Not sure what's going on with ensmallen or why poetry can't find it to install.

caufieldjh commented 2 years ago

There may be some strange python version interaction between poetry, ensmallen, and OS - the poetry install works perfectly on my machine, running Py3.9.5, but the workflow here is running Py3.9.13 on windows/linux. The windows one fails during install but the linux one works.

Going to try deactivating the windows tests as I don't think grape really supports it anyway.

caufieldjh commented 2 years ago

OK, now it makes it to pytest but just takes far too long. Probably trying to make a real embedding by accident.

caufieldjh commented 2 years ago

Tests are still taking too long - I suspect this test is the culprit: https://github.com/Knowledge-Graph-Hub/neat-ml/blob/424c3653fbf3f18e9a6510c9ea533c6e22f82913/tests/test_link_prediction.py#L95

caufieldjh commented 2 years ago

Hrm, looks like it may just need the extra imports to be test dependencies. Will try that.

caufieldjh commented 2 years ago

See https://github.com/AnacletoLAB/grape/issues/12 - just ran into this

caufieldjh commented 2 years ago

Looks like this is working - tests are still running long, but I think that's because it includes all the tensorflow and sklearn stuff that generally runs quickly (i.e., not 100 minutes) if it isn't on a GH actions instance.

caufieldjh commented 1 year ago

Hrm, the tests install TF and scikit-learn, but it's strange that they still take so long. On my machine it takes <2 min to run all tests.