Skip to content

Fixed comments from Christoph Buchner and Martin#8

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

Fixed comments from Christoph Buchner and Martin#8
SudeepLiu wants to merge 1 commit into
OpenModelica:masterfrom
SudeepLiu:master

Conversation

@SudeepLiu

Copy link
Copy Markdown

Fixed comments from Christoph Buchner and Martin

@bilderbuchi

Copy link
Copy Markdown

it is typically more useful to add this to the already existing PR #7 as an additional commit. this way discussion/remarks get preserved, and fixed section are more readily apparent.

edit: you can do this by sending a PR with the fixes to Alachew's master at https://github.com/alash325/OMPython/tree/master

@bilderbuchi

Copy link
Copy Markdown

also, regarding the comments: if all you do is repeat the function name, this it typically not very useful. It's better to (also) specify details about input arguments, returned objects, potential exceptions.
Additionally, please look up python docstrings and a pretty useful convention for functions if you add these - the comments you added right now are not parsed by Python and its IDEs, so is not really useful. I would understand if you don't bother, as the rest of the codebase, from a quick glance, is also not very much documented.

As a simple example, I used one of your member functions:

   #to get values of continuous quantities name
   def getContinuousValues(self, contiName=None):

better would be,e.g.,

  def getContinuousValues(self, contiName=None):
  """Return values of continuous quantities.

  If no quantity is given, XXX happens.

  Args:
    contiName (Optional[string]): Name of the continuous quantity. Defaults to None.
  Returns:
    list: Values of the continuous quantity queried.
  """

@bilderbuchi

Copy link
Copy Markdown

also, maybe, before investing this work, coordinate with @sjoelund if he intends to merge the (improved) work.

@alash325

Copy link
Copy Markdown

Import FMU is not working in your updated code i.e convertFmu2Mo. I think you removed the following function def requestApi__(self, apiName, entity ).

@SudeepLiu

Copy link
Copy Markdown
Author

Thanks you Alachew for reminding. I forgot that i have been calling that
function. Thats why i told to test by yourself. Well do more tests n let me
know abt more bug.

On Thursday, 14 April 2016, Alachew Mengist notifications@github.com
wrote:

Import FMU is not working in your updated code i.e convertFmu2Mo. I think
you removed the following function def requestApi__(self, apiName, entity
).


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#8 (comment)

With Regards,
Sudeep Bajracharya

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