Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/pypi packaging #501

Merged
merged 24 commits into from
Sep 1, 2020
Merged

Feature/pypi packaging #501

merged 24 commits into from
Sep 1, 2020

Conversation

Bachibouzouk
Copy link
Collaborator

@Bachibouzouk Bachibouzouk commented Aug 9, 2020

Fix #423
Fix #545

Changes proposed in this pull request:

  • Separate the installation of the packages needed for the report generation from the mvs simulation
  • Move all source files in srv/mvs_eland
  • Create src/mvs_eland/utils subpackage (contains constants.py, constants_json_string.py, constants_output.py.
  • Move the content of the previous src/utils.py module to src/mvs_eland/utils/__init__.py
  • Rename tests/constants.py --> tests/_constants.py
  • Refactor modules calls (mostly src. is replaced by mvs_eland.)
  • Remove tests/__init__.py
  • Delete mvs_eland_tool folder and move its content in src/mvs_eland
  • Update install_requires and extra_requires in setup.py
  • Fix failing KPI test (due to newer pandas version)
  • Gather all requirements files in a requirements folder and read the requirement from there for setup.py

The following steps were realized, as well (if applies):

  • Use in-line comments to explain your code
  • Write docstrings to your code
  • For new functionalities: Explain in readthedocs
  • Write test(s) for your new patch of code
  • Update the CHANGELOG.md
  • Apply black (black . --exclude docs/)
  • Check if benchmark tests pass locally (EXECUTE_TESTS_ON=master pytest)

For more information on how to contribute check the CONTRIBUTING.md.

@Bachibouzouk
Copy link
Collaborator Author

Needs to wait that #511 is merged

@Bachibouzouk Bachibouzouk added this to In progress in August tasks via automation Aug 17, 2020
@Bachibouzouk
Copy link
Collaborator Author

Bachibouzouk commented Aug 18, 2020

Not to forget this : #511 (comment)

@Bachibouzouk Bachibouzouk force-pushed the feature/pypi-packaging branch 2 times, most recently from 8919d90 to 07a4f4b Compare August 24, 2020 14:38
@Bachibouzouk
Copy link
Collaborator Author

Bachibouzouk commented Aug 25, 2020

Based on the recommendations of pytest I think the following setup

setup.py
src/
    mvs_eland/
        __init__.py
        A0_*.py
        ....
        F2_*.py
        utils/
            __init__.py
            constants.py
            ....
tests/
    test_A0_*.py
    ....

Seems the best structure to make sure the tests run on the installed package while it fixes the now weird import rules (currently in a python terminal under virtual env, from mvs_eland import constants raises an ImportError while from src import constants does not, the reverse would be better)

This would require to change from src.A0 into from mvs_eland.A0 and from src.constants into from mvs_eland.utils.constants. I made this changes locally and all our tests pass :)

If we do this, the code currently in mvs_eland_tool would be moved into src/mvs_eland. I would rename local_deploy.py to cli.py (for Command Line Interface).

This move has to be done before a release because it will change the code structure. I had not fully understood the pypi structure in the past when I introduced the src/ folder, I am sorry for that :(

@Bachibouzouk Bachibouzouk marked this pull request as ready for review September 1, 2020 15:47
Modify the README and readthedocs as well
@Bachibouzouk Bachibouzouk merged commit 3b855b5 into dev Sep 1, 2020
August tasks automation moved this from In progress to Done Sep 1, 2020
@Bachibouzouk Bachibouzouk deleted the feature/pypi-packaging branch September 1, 2020 20:57
@Bachibouzouk Bachibouzouk mentioned this pull request Sep 1, 2020
@Bachibouzouk
Copy link
Collaborator Author

@smartie2076, @SabineHaas, @Piranias, @mahendrark

The files that were in src are now in src/mvs_eland. After you pulled the changes you need to do pip install -e . to have the minimal requirements and pip install -e .[report] to be able to output the report. The files with the constants are now located in src/mvs_eland/utils. In the test files the imports have changed from from src.<module> to from mvs_eland.<module>. The package mvs_eland_tool does not exist anymore, I moved its files in the src/mvs_eland, local_deploy is now cli and server_deploy is now server. The tests constants tests/constants.py has been renamed tests/_constants.py. The automatic generation of the report now fails because the loading time necessary to output all figures has increased and the dash server times out, it was too late to debug, if you want to print the report run the simulation and then run python mvs_report.py -i <output folder of your simulation>/json_with_results.json (if you used default output path, you can simply run python mvs_report.py), the interactive report will be available at 127.0.0.1:8050 in a browser, you can then use the "print page" function of the browser. Note that the layout has been optimized for chrome browser. If you run into troubles, it might be the old local install of mvs called mvs-tool, you can pip uninstall mvs_tool or create a new virtual env (the cleaner solution). I have tested on windows and linux (windows I needed to reinstall graphviz like in the readthedocs troubleshooting).

@SabineHaas
Copy link
Collaborator

I started with a fresh virtual environment and installed all requirements via pip install -e .[report,docs,test] but somehow the modules are not found:

Traceback (most recent call last):
  File "/home/sabine/virtualenvs/mvs_elandversion2/mvs_eland/mvs_tool.py", line 1, in <module>
    from mvs_eland.cli import main
ModuleNotFoundError: No module named 'mvs_eland.cli'

Did you test the new setup in a clean environment and do you have any idea what could be the problem @Bachibouzouk?

These are all packages installed: packages.txt


Using the command line, however, it works.. at least I get up to the point where my input files are not up-to-date:

  File "/home/sabine/virtualenvs/mvs_elandversion2/mvs_eland/src/mvs_eland/D2_model_constraints.py", line 60, in add_constraints
    if dict_values[CONSTRAINTS][MINIMAL_RENEWABLE_SHARE][VALUE] > 0:
KeyError: 'constraints'

@SabineHaas
Copy link
Collaborator

@Bachibouzouk already solved - I show the solution here, in case any other ppl have the same issue:

So, I'm using Pycharm as IDE and I needed to uncheck the box "Add content roots to PYTHONPATH" in the "run/debug configurations" (open by Run - Edit Configuration from the menu).

This wasn't needed before the changes and I think it has something to do with a specific folder being chosen as source now..? But actually I have no idea and I'd like to understand - so if you have an idea it would be cool if you could explain @Bachibouzouk :)

@Bachibouzouk
Copy link
Collaborator Author

I always run code within my terminal and not within PyCharm IDE. I did the tests in a fresh environment. I am not sure why this happened to you...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
August tasks
  
Done
2 participants