lk-geimfari / mimesis

Mimesis is a robust data generator for Python that can produce a wide range of fake data in multiple languages.
https://mimesis.name
MIT License
4.44k stars 336 forks source link

Full refactoring (restarting) of the project #175

Closed lk-geimfari closed 6 years ago

lk-geimfari commented 7 years ago

We need to refactor this project. Completely.

Goals:

lk-geimfari commented 7 years ago

We will try to do this all by the end of the summer, given that we only work on this project on weekends.

sobolevn commented 7 years ago

I also advise you to add flake8. Here's some must-have plugins:

  1. flake8-builtins
  2. flake8-commas
  3. flake8-quotes (my fav)
  4. isort (not actually a flake8-plugin, but still very useful)

Here's the full list: https://github.com/sobolevn/django-split-settings/blob/master/setup.py#L26

sobolevn commented 7 years ago

About your points.

  1. Remove useless providers/methods. Which ones do you consider useless? Are you sure that noone uses it?
  1. Change the structure of the project. ++

  2. Bring the code in compliance with the requirements of PEP8. ++

  3. Add/Update documentation. ++

  4. Add auto deploy on PyPi on tag. ++

  5. Add mechanism for uglify json files (this will reduce the file size). ++

  6. Add mechanism for inflect Russian first, last, and middle names (great example is petrovich). We can realize this like a decorator, maybe. I guess this should be done separately. Do you still want to have no dependencies?

lk-geimfari commented 7 years ago

@sobolevn I think that json() method of Structure() is too specific and useless. Also i removed point 7. We do not need dependencies. I'll update goals.

sobolevn commented 7 years ago
valerievich commented 7 years ago

Few questions about "Bring the code in compliance with the requirements of PEP8 (flake8, pycodestyle)":

  1. Should we check only *.py files or *.json too?
  2. There are a lot of F401, F403, F405 errors (produced Flake8). It's because of from somemodule import *. Is it better to ignore these errors?
  3. In /int/file.py we have many lines with error E501. But if resolve them code starts to look uglier.

screen shot 2017-06-21 at 5 59 22 pm In my opinion in this case we could not edit these lines. “Beautiful is better than ugly”

lk-geimfari commented 7 years ago

@Valerievich

  1. We should always ignore data/* because in data/ we store only data, not code of mimesis.
  2. Ignore.
  3. Point 1. We ignore int/, because is data.
valerievich commented 7 years ago

In /test_data/test_generic.py we have:

def test_bad_argument(generic):
    with pytest.raises(AttributeError):
        result = generic.bad_argument

This causes F841 local variable 'result' is assigned to but never used

Is this wrong code? Have I to add an assert?

lk-geimfari commented 7 years ago

@Valerievich You can replace result with __. It's not wrong code, we use context manager here, which except errors.

valerievich commented 7 years ago

How to understand which providers/methods are useless (it's about first task of list)? Are there conditions?

lk-geimfari commented 7 years ago

@Valerievich for example json() of Structured(). It was bad idea to get data for result of this method using Personal(). Try to replace it with random json generator with random structure and with random deep.

valerievich commented 7 years ago

@lk-geimfari Should there be clear data? Or is it possible to generate data smth like {'key1': value1, 'key2': value2 ...} ?

lk-geimfari commented 7 years ago

@Valerievich Actually useful json() generator was been already created by @wikkiewikkie. It was a mistake to propose him to replace that method with a new one. Here you can look at old implementation of json(). Use it. Just refactor. Also here you can find tests of this implementation.

valerievich commented 7 years ago

I've done with Structured() #187. Is it necessary to remove another Providers or methods? I can help.

lk-geimfari commented 7 years ago

@Valerievich Actually you can help us with much important things. If you are use PyCharm you can find TODO list.

For example: https://github.com/lk-geimfari/elizabeth/blob/feature-new-strucutre/tests/conftest.py#L56

Text() is international provider (i.e support all locales), but at this moments we test only default locale (default locale is en).

Help us with tasks in TODO. All the main work needs to be done in a new branch: feature-new-strucutre.

valerievich commented 7 years ago

ToDo "Resolve importing errors and use importing format as above" is secret for me. Can't resolve importing errors. Where should I look? Any ideas?

sobolevn commented 7 years ago

@Valerievich what's your exact problem? What kind of import errors do you have? Could you please provide more information?

valerievich commented 7 years ago

I try to use following import format from mimesis.providers import Games in generic.py

and then get this: tr.txt

lk-geimfari commented 7 years ago

@Valerievich I get similar error. I don't know why it's happens, but we really need to fix it.

sobolevn commented 7 years ago

@Valerievich could you please try this:

from mimesis.providers.games import Games
from mimesis.providers.hardware import Hardware

Instead of:

# TODO: Resolve importing errors and use importing format as above.
from .games import Games
from .hardware import Hardware
valerievich commented 7 years ago

@sobolevn I tried this way. Import worked well in this case. But @lk-geimfari wants to use format exactly like from mimesis.providers import Games. There are not any problem with import other providers in generic.py e.g. Address .

valerievich commented 7 years ago

@lk-geimfari I've resolved that import problem issue in new PL #198 Please check it.

lk-geimfari commented 7 years ago

I have added json minify, but the result is quite the opposite of what I expected. The size of the project even increased from 6.8 Mb to 7.2 Mb. If no one has any idea why this happens, then I suggest removing the minifer.

valerievich commented 7 years ago

@lk-geimfari why do you think it's unacceptable ? 0.4Mb is not a big increase I think.

lk-geimfari commented 7 years ago

@Valerievich The goal was to reduce the size of the project, by minimizing the JSON files, but it worked exactly the opposite. What is a reason to minimizing of JSON files if the size of the project was only become larger? Don't you agree?

lk-geimfari commented 7 years ago

@Valerievich 6.8 Mb is size before minimize and 7.2 Mb is size after minimize.

valerievich commented 7 years ago

I think 'Add/Update documentation.' is done.

lk-geimfari commented 7 years ago

@Valerievich It's still do not work: http://mimesis.readthedocs.io/en/latest/api.html#cryptographic. I don't know why.

valerievich commented 7 years ago

How did you create this version? All that necessary is go to /docs and exec. make html. I've pulled master branch recently and it works for me.

screen shot 2017-09-11 at 12 57 21 pm
lk-geimfari commented 7 years ago

@Valerievich It's created automatically by ReadTheDocs. I'll try to do it manually.

sobolevn commented 7 years ago

I have faced some issues while using rtd. Check its logs and configs.

lk-geimfari commented 7 years ago

@sobolevn Seems like i have incorrectly configured rtd. I'll fix it tonight.

lk-geimfari commented 7 years ago

@sobolevn Do you have an account on rtd? Can i add you to maintainers on rtd?

lk-geimfari commented 7 years ago

@sobolevn Please, check out your rtd account. I have added you to maintainers. Thank you!

lk-geimfari commented 7 years ago

@sobolevn And i have deleted your answer above, because sometimes people grab emails on github for spamming.

lk-geimfari commented 7 years ago

@sobolevn Can you look at settings on the ReadTheDocs, please. I don't undersand why, but on my local machine and in rtd the documentation generates without API Refernce. Thanks!

valerievich commented 7 years ago

Find and fix bottlenecks in Generic(). Just run this script and you'll understand what i talking about @Valerievich.. I think this is a result of call built-in method def seed() from random.Random. We call it from our custom Random() class in helpers.py.

ifiokeyo commented 7 years ago

I would love to contribute to this. How do I get involved.

lk-geimfari commented 7 years ago

@andela-ieyo Please, look at the last task in TODO. Thanks!

CP-NEMO commented 7 years ago

include

sobolevn commented 6 years ago

Hooray! 🎉

lk-geimfari commented 6 years ago

@sobolevn Yeah, we did it! :smile: