search

OpenSourceEconomics / grmpy

Python package for the simulation and estimation of generalized Roy model
http://grmpy.readthedocs.io
MIT License
19 stars 5 forks source link

Codacy Review #90

Closed peisenha closed 6 years ago

peisenha commented 6 years ago

There are some easy wins, can we get it to an A before the class. https://www.codacy.com/app/eisenhauer/grmpy/issues/index?&filters=W3siaWQiOiJDYXRlZ29yeSIsInZhbHVlcyI6WyJDb2RlIFN0eWxlIl19XQ==

SeBecker commented 6 years ago

I fixed some smaller issues (in the sebecker_bugfixes branch), but i have no idea what i have to change so that codacy doesn't want to change the tests into functions. Maybe you have to ignore the issues manually?

peisenha commented 6 years ago

Looking here, there are still some easy wins: https://www.codacy.com/app/eisenhauer/grmpy/issues/index?bid=6279798&filters=W3siaWQiOiJDYXRlZ29yeSIsInZhbHVlcyI6WyJDb2RlIFN0eWxlIl19XQ==

Remove unused variables, replace assert by numpy.testing.assert_*, remove trailing whitespaces

Regarding the test warnings, restructure all our tests like here https://github.com/briqInstitute/interalpy/blob/master/interalpy/tests/test_integration.py, where there is no class layer around it ... Make sure that the fixtures that we defined for the class level are now called upon at the module level. See here https://github.com/briqInstitute/interalpy/blob/master/interalpy/tests/conftest.py

On Mon, Jan 15, 2018 at 8:39 PM, Sebastian Becker notifications@github.com wrote:

I fixed some smaller issues (in the sebecker_bugfixes branch), but i have no idea what i have to change so that codacy doesn't want to change the tests into functions. Maybe you have to ignore the issues manually?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/grmToolbox/grmpy/issues/90#issuecomment-357771987, or mute the thread https://github.com/notifications/unsubscribe-auth/ADcMJzqpCa4CyFD968UReV1xyt-KPmWAks5tK6l7gaJpZM4ReCXE .

-- Philipp Eisenhauer Economist

Mail eisenhauer@policy-lab.org Web www.eisenhauer.io Repository https://github.com/peisenha

SeBecker commented 6 years ago

I fixed nearly all codacy issues, but 3 remaining security issues. Codacy complains even if I use subprocess.check_call instead of os.system. I have to find an other solution for this.

peisenha commented 6 years ago

That is fine, leave as is.

peisenha commented 6 years ago

This is done somewhere ...

peisenha commented 6 years ago

https://www.codacy.com/app/eisenhauer/grmpy/dashboard?bid=6279798 You can now directly review the codacy report for your branch ... Let me know when you renamed your branch, then I will set it up properly again.

SeBecker commented 6 years ago

After several try and error commits, sebecker_bugfixes is back on an A level ;-) https://app.codacy.com/app/eisenhauer/grmpy/dashboard?bid=6279798