Mimesis: Full refactoring (restarting) of the project

Created on 10 Jun 2017  ·  42Comments  ·  Source: lk-geimfari/mimesis

We need to refactor this project. Completely.

Goals:

  • [x] Remove useless providers/methods.
  • [x] Change the structure of the project.
  • [x] Bring the code in compliance with the requirements of PEP8.
  • [x] Add/Update documentation.
  • [x] Add auto deploy on PyPi on tag.
  • [x] Add mechanism for uglify json files (this will reduce the file size) only when deploy on PyPi, otherwise we can't normally edit json files when will update provider's data.
  • [ ] Find and fix bottlenecks in Generic(). Just run this script and you'll understand what i talking about.
enhancement

Most helpful comment

Hooray! 🎉

All 42 comments

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

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

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?

@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.

  • Type hint everything ?

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”

@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.

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?

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

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

@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.

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

@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.

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

@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.

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?

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

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

and then get this:
tr.txt

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

@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

@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 .

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

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.

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

@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?

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

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

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

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

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

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

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

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

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

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

@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!

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.

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

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

include

Hooray! 🎉

@sobolevn Yeah, we did it! :smile:

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kairichard picture kairichard  ·  6Comments

lk-geimfari picture lk-geimfari  ·  3Comments

lk-geimfari picture lk-geimfari  ·  4Comments

chris-canipe picture chris-canipe  ·  5Comments

dumprop picture dumprop  ·  6Comments