Skip to content
This repository has been archived by the owner on Jul 8, 2024. It is now read-only.

Python module documentation #142

Merged
merged 6 commits into from
Nov 8, 2019
Merged

Python module documentation #142

merged 6 commits into from
Nov 8, 2019

Conversation

mikhailnikolaev
Copy link
Collaborator

@mikhailnikolaev mikhailnikolaev commented Oct 24, 2019

Documentation about adding and testing Python modules is added.

Note: reference to global runner documentation from the file /docs/python/Modules.md is currently broken, because such a documentation is in the PR #153

Copy link
Collaborator

@tomjaguarpaw tomjaguarpaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks nice.

Returns an object of the class `<Objective>Output` contains calculated values.
Such classes are defined int the files `<Objective>Data.py`.

4. Do not forget to add your module to the global runner script.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By "global runner" do you mean run-all.ps1? If so could you say so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be a reference to section describes what the global runner is (from Architecture.md). So, what do you think about the new version?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is good.

Copy link
Collaborator

@iliaeg iliaeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to write about adding CMakeLists and python requirements for new modules.
E.g.

  • Add the following line to /src/python/modules/CMakeLists.txt:
    add_subdirectory ("<ModuleName>")
  • Add to a new module directory
    1. requirements.txt with python requirements of a new module, e.g.
    scipy>=1.3.1
    setuptools>=41.0.0
    
    1. CMakeLists.txt with something like following
    project(<ModuleName>)
    
    execute_process(
        COMMAND ${Python3_EXECUTABLE} "-m" "pip" "install"
        "-r" "${CMAKE_CURRENT_SOURCE_DIR}/requirements.txt"
        )
    This command install required pip packages on configure stage of CMake.

docs/python/Modules.md Outdated Show resolved Hide resolved
docs/python/Modules.md Outdated Show resolved Hide resolved
docs/python/Modules.md Outdated Show resolved Hide resolved
@iliaeg
Copy link
Collaborator

iliaeg commented Nov 5, 2019

@mikhailnikolaev if you think that adding python requirements is beyond of the scope of module documentation then I should add it to docker docs.

@mikhailnikolaev
Copy link
Collaborator Author

@mikhailnikolaev if you think that adding python requirements is beyond of the scope of module documentation then I should add it to docker docs.

Actually, I think that this stuff is about Docker, not about the Python modules. So, maybe, it is more proper to add it in a Docker docs

@iliaeg
Copy link
Collaborator

iliaeg commented Nov 5, 2019

@mikhailnikolaev if you think that adding python requirements is beyond of the scope of module documentation then I should add it to docker docs.

Actually, I think that this stuff is about Docker, not about the Python modules. So, maybe, it is more proper to add it in a Docker docs

Okay, I had doubts about this, I will do so.

@msdmkats
Copy link
Contributor

msdmkats commented Nov 6, 2019

@mikhailnikolaev if you think that adding python requirements is beyond of the scope of module documentation then I should add it to docker docs.

Actually, I think that this stuff is about Docker, not about the Python modules. So, maybe, it is more proper to add it in a Docker docs

I disagree. First, this is about fetching dependencies as a part of CMake configuration/build. Second, this is supposed to be a guide on adding a new python module to ADBench. So, as a developer I would expect, that following steps listed here should be enough to do so. But without CMakeLists that won't be the case. So, I suggest, adding Ilia's paragraph to this doc.

@mikhailnikolaev mikhailnikolaev merged commit df2d454 into master Nov 8, 2019
@mikhailnikolaev mikhailnikolaev deleted the miniko/pydoc branch November 8, 2019 12:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants