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

Randomly apply manipulator when applying manipulator with limit #194

Closed
seongahjo opened this issue Feb 11, 2022 · 4 comments · Fixed by #230
Closed

Randomly apply manipulator when applying manipulator with limit #194

seongahjo opened this issue Feb 11, 2022 · 4 comments · Fixed by #230
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Milestone

Comments

@seongahjo
Copy link
Contributor

seongahjo commented Feb 11, 2022

ArbitraryNodes found by expression has fixed ordering.
So applying manipulator with parameter limit, which parameter used as limiting the number of applied ArbitraryNode, always returns same applied ArbitraryNode.
It is not an intuitive result.

In short, this issue is to shuffle all ArbitraryNodes found by ArbitraryExpression.


적용 회수를 입력받는 연산 (limit 을 입력받는 연산)에서 조회한 노드 순서대로 적용하는 게 아니라 랜덤하게 적용하도록 수정합니다.

@seongahjo seongahjo added the help wanted Extra attention is needed label Feb 11, 2022
@seongahjo seongahjo added this to the 1.0.0 milestone Feb 11, 2022
@seongahjo seongahjo changed the title Randomly apply manipulator Randomly apply manipulator when applying limited manipulator Feb 11, 2022
@seongahjo seongahjo changed the title Randomly apply manipulator when applying limited manipulator Randomly apply manipulator when applying manipulator with limit Feb 11, 2022
@seongahjo seongahjo added the good first issue Good for newcomers label Feb 11, 2022
@sandrawangyx
Copy link
Contributor

Hi @seongahjo, I'm interested in working on this issue.
if it is ok with you, do you mind putting the description in English when you get a chance?
Thank you!

@seongahjo
Copy link
Contributor Author

@sandrawangyx
Yes, sure.
Thank you for paying attention to good first issue

This issue could be translated as below.

ArbitraryNodes found by expression has fixed ordering.
So applying manipulator with parameter `limit`, which parameter used as limiting the number of applied ArbitraryNode, always returns same applied ArbitraryNode.
It is not an intuitive result.

In short, this issue is to shuffle all `ArbitraryNodes` found by `ArbitraryExpression`.

If you have any question, feel free to ask!
Thank you

This was referenced Mar 22, 2022
@sandrawangyx
Copy link
Contributor

Hi @seongahjo thanks for the comment.
My initial thought on the approach is to re-arrange the tree node.
Let me know if I am on the right track.
we should update the arbitrary tree whenever we update it, is this correct?

@seongahjo
Copy link
Contributor Author

seongahjo commented Mar 25, 2022

@sandrawangyx
yes, but I think you should not need to rearrange nodes on tree actually.

Because there are some situations below.
set("*.*", "value" , 5)
"*.*" is hard to refer on tree.

In addition, it is no need to maintain same order.

So I think rearranging nodes found by expression is better.
Feel free to share other opinions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants