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

Pylint changes #327

Merged
merged 1 commit into from
Jun 15, 2020
Merged

Pylint changes #327

merged 1 commit into from
Jun 15, 2020

Conversation

5iver
Copy link
Member

@5iver 5iver commented Jun 15, 2020

Signed-off-by: Scott Rushworth [email protected]

Signed-off-by: Scott Rushworth <[email protected]>
@5iver 5iver merged commit 65ef09f into openhab-scripters:master Jun 15, 2020
@besynnerlig
Copy link
Contributor

I've reviewed an already merged PR. I'm not sure that you will be notified and aware about my question above @openhab-5iver

@5iver
Copy link
Member Author

5iver commented Jun 21, 2020

Unfortunately, I do not see any comment or review. They do show up after merge. What was your question?

@besynnerlig
Copy link
Contributor

@openhab-5iver ,

Unfortunately, I do not see any comment or review. They do show up after merge. What was your question?

In Core/automation/lib/python/core/utils.py
LOG.warn("The 'core.utils.getItemValue' function is pending deprecation.") and similar ...

I'm getting a lot of warnings in my log. What is the reason for the pending deprecation?

value

Returns:
int, float, ON, OFF, OPEN, CLOSED, str, DateTime, or None: the state if
the Item converted to the type of default value, or the default
value if the Item's state is NULL or UNDEF
"""
LOG.warn("The 'core.utils.getItemValue' function is pending deprecation.")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm getting a lot of warnings in my log. What is the reason for the pending deprecation?

@besynnerlig
Copy link
Contributor

I'm not sure if you get notifications for this @openhab-5iver due to that it's closed so I post this and mention you. (Sorry if I spammed you)

@5iver
Copy link
Member Author

5iver commented Jun 24, 2020

Hey @besynnerlig! No worries... I've just been busy with other things! I marked several of these functions deprecated because I'd like to remove them. I had mentioned this when they were first included, but I probably should have said something to you before merging this. My reasoning is that I am preparing to recreate the helper libraries in the rule engine and this library will be the first to go. Most of these functions I think would be better documented as an example or in 'But How Do I?'. Specifically...

  • kw: This is very specialized and I don't think it will be missed. Maybe something for "But How Do I"?
  • iround: Instead, use int(float(str(round(test_number)))).
  • getItemValue: This is a specialized function and in most cases items["My_Item"] works fine for getting the state of an Item. This would be better as an example.
  • getLastUpdate: This is another specialized function that would be better as an example.
  • sendCommand and postUpdate: These just eliminate the need for prepending events on these methods. People should learn to use events or we should remove it altogether.
  • post_update_if_different and send_command_if_different: I think these have proven useful for migration to the rule engine by adding a boolean parameter to the actions for whether to compare to the existing value or not. Like events.sendCommand("My_Item", "ON", True) and events.sendCommand("My_Number_Item", "5.5", True, 1) (the later includes floatPrecision).

@besynnerlig
Copy link
Contributor

However the kw function was changed in such a way that the for loop now alters the argument hence renders wrong results. I'm not sure if I need to report that as a new issue or if it's enough to mention it here.

The problem is that the function argument name value is reused in the for loop.

@5iver
Copy link
Member Author

5iver commented Jul 3, 2020

Oh my... fixed (#346)!

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.

2 participants