Skip to content

- Sudeep's thesis work OMPython Enhancement - #7

Closed
alash325 wants to merge 1 commit into
OpenModelica:masterfrom
alash325:master
Closed

alash325 wants to merge 1 commit into
OpenModelica:masterfrom
alash325:master

Conversation

@alash325

@alash325 alash325 commented Apr 6, 2016

Copy link
Copy Markdown

No description provided.

Comment thread OMPython/__init__.py
import pandas as pd
import csv

import matplotlib.pyplot as plt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it wise to depend on matplotlib when it causes the compiler to use exponential memory? https://trac.openmodelica.org/OpenModelica/ticket/3764

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

probably it can be dropped, as it's only used in a very thin wrapper around plt.plot. Same with the other dependencies, pd is only used a couple of times to return some pd dataframes (maybe the user should do that instead if desired)´, and numpy is used only twice (although one is an interp, so may be difficult to replace).

@bilderbuchi

Copy link
Copy Markdown

I had a quick glance over this, but stopped reviewing at about L1030.
This PR seems to use Tabs for indentation, as opposed to the rest of the file (afaict), and basically the rest of Python, might be wise to change this before merging.
Also, 700+ lines of diff, and not a single explanatory comment, or class/method/function docstring, and no tests? I really wouldn't want to maintain this tbh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants