Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Improved the Scale Transformation article. #4443

Merged
merged 5 commits into from Oct 27, 2017
Merged

Improved the Scale Transformation article. #4443

merged 5 commits into from Oct 27, 2017

Conversation

Confectrician
Copy link
Contributor

@Confectrician Confectrician commented Oct 19, 2017

This PR is moved from openHAB documentation.

Main discussion took part in:
openhab/openhab-docs#486

We already discussed there about a more handy example in the future,
but there are no details yet, so i wanted to finish this reworking on the current state.

For the record:

Related: #4439

Closes openhab/openhab-docs#486
Closes openhab/openhab-docs#484

Since this is my first PR on a eclipse repo, feel free to comment if there is something missing.
I have done all the steps from the contribution guidelines.
(Eclipse Account, GitHub Settings in there, Contributor Agreement)
Most things should be ok this way.

Signed-off-by: Jerome Luckenbach github@luckenba.ch (github: @Confectrician)

Main discussion took part in:
openhab/openhab-docs#486

Signed-off-by:  Jerome Luckenbach <github@luckenba.ch> (github: @Confectrician)
Signed-off-by:  Jerome Luckenbach <github@luckenba.ch>
Copy link
Contributor

@sjsf sjsf left a comment

Choose a reason for hiding this comment

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

Really great improvement, thanks!!! 👍

I have to admit that without your changes I didn't really get what this transformation is about.

I'm even thinking if it would make sense to mention the term "discretization" somewhere, although I'm not exactly sure whether it mathematically is fully correct or not.

One additional thought: Do you think it would make sense to explain the "inclusive" and "exclusive" bracket notations a little (for the less savvy users)? I'm not sure if that becomes clear if somebody never heard of that...

@@ -1,16 +1,51 @@
# Scale Transformation Service

Transform the input by matching it between limits of ranges in a scale file. The input string must be in numerical format.
Transform a given input by matching it between limits of ranges.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know, you just broke the lines here, but would you agree that "Transform a given input by matching it to ranges" is less confusing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree.

Wdyt about:
"Transform a given input by matching it to specific ranges."
or
"Transform a given input by matching it to specified ranges."

Just to make directly clear, that there are no fixed ranges somewhere in esh that are used for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 "specified"


Each value the item receives, will be categorized to one of the five given ranges.
Values **lower than or equal to 3** are caught with `[..3]=1`.
Greater values are catched in ranges with 2 values as criteria.
Copy link
Contributor

Choose a reason for hiding this comment

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

are catched -> "are caught" (or maybe rather "are matched")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think "matched" it will be, when I compare the options.
That sounds/reads the best. (My opinion)

Each value the item receives, will be categorized to one of the five given ranges.
Values **lower than or equal to 3** are caught with `[..3]=1`.
Greater values are catched in ranges with 2 values as criteria.
The only condition here is that the received value has to be lower than or equal to `100` in our example, since we haven't defined other cases yet.
Copy link
Contributor

Choose a reason for hiding this comment

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

lower or equal to than

Signed-off-by: Jerome Luckenbach <github@luckenba.ch>
@Confectrician
Copy link
Contributor Author

I'm even thinking if it would make sense to mention the term "discretization" somewhere, although I'm not exactly sure whether it mathematically is fully correct or not.

That would be something for an opener sentence.
I'll look if i can create a useful sentence for that.

One additional thought: Do you think it would make sense to explain the "inclusive" and "exclusive" bracket notations a little (for the less savvy users)? I'm not sure if that becomes clear if somebody never heard of that...

Good point. I have thought about that too already.
We discussed in the openHAB PR already about a better understandable example.
I think, this should be combined then.
The explanation on its own may be confusing too.
I would prefer an updated example with a matching explanation on a later point.

Wdyt?

Signed-off-by: Jerome Luckenbach <github@luckenba.ch>
@sjsf
Copy link
Contributor

sjsf commented Oct 19, 2017

I would prefer an updated example with a matching explanation on a later point.

Sure, there is no pressure to do it here in this PR. This is a huge improvement as it is now!

[For the record: The automatic IP validation fails because of the edit directly within Github. I manually verified that your ECA is valid, so that's fine.]

@Confectrician
Copy link
Contributor Author

Hm not sure, but a first try:

"The Scale Transformation Service is a an easy to handle tool, that can help you with the discretization of number inputs."

second sentence would then be changed to something like this:
"It transforms a given input by matching it to specified ranges."

@sjsf
Copy link
Contributor

sjsf commented Oct 20, 2017

Yes, for me this totally makes sense. I like it!
From my point of view we you may update the PR accordingly (just drop the comma) and then it's ready to be merged.

… sentence.

Signed-off-by: Jerome Luckenbach <github@luckenba.ch>
@Confectrician
Copy link
Contributor Author

Here we go. Update is done.

One off topic thing for my understanding:

[For the record: The automatic IP validation fails because of the edit directly within Github. I manually verified that your ECA is valid, so that's fine.]

Did this fail due to the missing sign off within the PR message?
Just as a reminder for me to do it better next time. :)

@sjsf sjsf merged commit e1ad40a into eclipse-archived:master Oct 27, 2017
@sjsf
Copy link
Contributor

sjsf commented Oct 27, 2017

Thanks!

Did this fail due to the missing sign off within the PR message?

No, they were fine (but yes, a missing sign-off line also makes it fail). The problem is that github's web editor created commits with itself as the committer and not you. So the only way around it would be not using the web editor. However, doing the lookup manually really is nothing more than two clicks and a c'n'p of your email address. I'm very happy to do that manually if that helps setting the barrier as low as possible for improving our documentation.

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.

None yet

3 participants