petervizi / python-eeml

A python package for generating eeml documents.
http://petervizi.github.com/python-eeml
GNU General Public License v3.0
43 stars 11 forks source link

Added a way to control ordering (inserting numbers) into a channel #29

Closed idcrook closed 7 years ago

idcrook commented 11 years ago

Included updating the built-in tests

petervizi commented 11 years ago

Hey dpcrook,

Two minor comments:

  1. Could you please describe a use case for this new parameter?
  2. I would prefer this new optional parameter to be the last one, that way it won't brake existing code:
def __init__(self, id_, value, tags=list(), minValue=None, maxValue=None, unit=None, at=None, datapoints=None, id_name=None):

Thanks, Peter

idcrook commented 11 years ago

Hello Peter.

Thanks for sharing your excellent eeml library for python.

As a use case: I have used it so I can enumerate the channel names, and include strings in their names. This is helpful to provide a more descriptive name along with keeping a sortable order.

And example using the code can be found here:

https://xively.com/feeds/123164

where you may note that the first four fields are uploaded with a number and named field, and appear 0,1,2,3 (screenshot attached)...

[image: Inline image 2]

On your second comment, I have no reason on why it could not be placed at the end of the named arg list... I had not really consider the case of unnamed arguments, but instead was thinking of putting it next to the closely related argument. I was trying to have as little impact to the existing API as possible, as well as preserve backwards compatibility. Putting it at the end of the arg list would do an even better job of that.

Thanks, David

On Tue, May 21, 2013 at 2:46 PM, Peter Vizi notifications@github.comwrote:

Hey dpcrook,

Two minor comments:

  1. Could you please describe a use case for this new parameter?
  2. I would prefer this new optional parameter to be the last one, that way it won't brake existing code:

def init(self, id_, value, tags=list(), minValue=None, maxValue=None, unit=None, at=None, datapoints=None, id_name=None):

Thanks, Peter

— Reply to this email directly or view it on GitHubhttps://github.com/petervizi/python-eeml/pull/29#issuecomment-18239423 .

idcrook commented 11 years ago

Hello Peter.

Thanks for sharing your excellent eeml library for python. Doing an email reply seems to have biffed the formatting.

As a use case: I have used it so I can enumerate the channel names, and also include descriptive strings in their names. This is helpful to provide a more descriptive name along with keeping a sortable order.

And example using the code can be found here:

https://xively.com/feeds/123164

where you may note that the first four fields are uploaded with a number and named field, and appear 0,1,2,3 ...

image

On your second comment, I have no reason on why it could not be placed at the end of the named arg list... I had not really consider the case of unnamed arguments, but instead was thinking of putting it next to the closely related argument. I was trying to have as little impact to the existing API as possible, as well as preserve backwards compatibility. Putting it at the end of the arg list would do an even better job of that.

Did you want me to make that edit, and then re-request a pull?

Thanks, David

idcrook commented 7 years ago

Closing this old pull request