MantisAI / nervaluate

Full named-entity (i.e., not tag/token) evaluation metrics based on SemEval’13
MIT License
154 stars 19 forks source link

FIX: enforce code quality/formatting and typing + change to pyproject.toml #62

Closed davidsbatista closed 1 year ago

davidsbatista commented 1 year ago

So, here is the BIG PR:

├── LICENSE
├── README.md
├── pyproject.toml
├── setup.cfg
├── src
│   └── nervaluate
│       ├── __init__.py
│       ├── nervaluate.py
│       └── utils.py
├── tests
│   ├── __init__.py
│   ├── test_evaluator.py
│   ├── test_loaders.py
│   ├── test_nervaluate.py
│   └── test_utils.py
└── tox.ini

TESTS:

NOTE:

davidsbatista commented 1 year ago

@ivyleavedtoadflax regarding the virtualenv, my bad - I completely forgot to test that one, since I just created a virtual_env install the required dependencies and did all the development there - I will fix it.

Also, the pre-commit is a good idea, makes sense to make the quality code enforcing.

Thanks for your quick feedback.

davidsbatista commented 1 year ago

@ivyleavedtoadflax a suggestion:

Following the same philosophy that I wrote above of running the tests against the development version , why not just make sure the development version we have is on the PYTHONPATH, then one can develop and test/call functions directly without having to install the package after each change in the source code.

Does this makes sense to you ? or I'm missing the whole python of the setup.py step inside the virtualenv on the Makefile?

nsorros commented 1 year ago

@ivyleavedtoadflax a suggestion:

Following the same philosophy that I wrote above of running the tests against the development version , why not just make sure the development version we have is on the PYTHONPATH, then one can develop and test/call functions directly without having to install the package after each change in the source code.

Does this makes sense to you ? or I'm missing the whole python of the setup.py step inside the virtualenv on the Makefile?

I guess a contributor would have to manually change the PYTHONPATH during setup whereas with setup.py develop this happens during the creation of virtualenv so when someone makes changes they are reflected in the same way as setting the PYTHONPATH. So I would argue that is a better flow. Is there any reason for PYTHONPATH that I am missing?

davidsbatista commented 1 year ago

I'm confused with what the setup.py does - isn't it installing the code as a package locally?

If that's the case, this means that for a developer to see changes to code reflected in the current environment he needs to run setup.py again, right?

When I develop I usually create a new env, and then add to the PYTHONPATH the package I'm working on and every time I do import <package> whatever changes I did are already reflected.

nsorros commented 1 year ago

I'm confused with what the setup.py does - isn't it installing the code as a package locally?

If that's the case, this means that for a developer to see changes to code reflected in the current environment he needs to run setup.py again, right?

When I develop I usually create a new env, and then add to the PYTHONPATH the package I'm working on and every time I do import <package> whatever changes I did are already reflected.

python setup.py develop is equivalent to an editable install which what I usually do. And editable install allows you to change the code and rerun without having to update your pythonpath to have the changes reflected. Check this https://setuptools.pypa.io/en/latest/userguide/development_mode.html

davidsbatista commented 1 year ago

Thanks for the pointers @nsorros I've never actually used such a feature from the setuptools, and mostly relied more on PyCharm and/or PYTHONPATH

so, I've solved it by running this:

source .venv/bin/activate && pip3 install --editable .

and the pre-commit now runs code quality-checks:

check yaml...............................................................Passed
black....................................................................Passed
pylint...................................................................Passed
flake8...................................................................Passed
mypy.....................................................................Passed
- hook id: mypy
- duration: 0.13s

Success: no issues found in 3 source files

I can add the tests in a next PR.