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

Bug: Deriver should take over feature and label space from the first instance #1031

Closed
3 tasks
detlefarend opened this issue Jul 13, 2024 · 2 comments · Fixed by #1032
Closed
3 tasks

Bug: Deriver should take over feature and label space from the first instance #1031

detlefarend opened this issue Jul 13, 2024 · 2 comments · Fixed by #1032
Assignees
Labels
BF Basic Functions/Infrastructure bug Something isn't working

Comments

@detlefarend
Copy link
Member

1 Describe the bug
The class bf.streams.tasks.Deriver carries out an early definition of it's internal feature and label space in the constructor. Especially, class bf.math.Set is used as the underlying set (see lines 96,111). The problem with it occurs in stream tasks that carry out spatial distance computations using feature_data.get_related_set().distance() (e.g. oa.streams.tasks.clusteranalyzers.clusters.centroid.py, line 96). To do this, the instance should provide a metric space with an implementation of the mentioned method. The consequence is that the stream class needs to provide proper metric spaces. The processing tasks, in turn, should take over this space from the first instance. That was the reason for one of my recent refactorings in class Rearranger and all stream classes (exchanged Set() by ESpace()).

Long story short: the Deriver should set up the internal feature and label spaces based on the sets of the first instance (see Rearranger).

howto_bf_streams_131_stream_task_deriver.py can be used to fix this problem. The howto shows a further problem also: in lines 94 and 110, there is an illegal access to protected attributes. Since Rearrange has been refactored, protected attribute _feature_space is no longer initialized after creation of an object. An alternative specification of features to be derived is needed.

2 To Reproduce
Steps to reproduce the behavior:

  1. Run howto_bf_streams_131_stream_task_deriver.py

3 Expected behavior
A clear and concise description of what you expected to happen.

4 Screenshots
If applicable, add screenshots to help explain your problem.

5 Additional Informations

5.1 Operating System

  • Linux
  • Windows
  • Mac OS
@detlefarend detlefarend added bug Something isn't working BF Basic Functions/Infrastructure next release labels Jul 13, 2024
steveyuwono added a commit that referenced this issue Jul 17, 2024
steveyuwono added a commit that referenced this issue Jul 17, 2024
steveyuwono added a commit that referenced this issue Jul 17, 2024
@steveyuwono
Copy link
Collaborator

Hi @detlefarend , I have updated the deriver in the branch "bf/oa/streams/refact." To review the changes, refer to the pull request "OA: Refactoring Streams #1032." This includes updates to the Deriver class, its related documentation, and its class diagram. If you have time, please review the changes and provide feedback on any areas that may need improvement. Thank you!

@detlefarend
Copy link
Member Author

Hi @detlefarend , I have updated the deriver in the branch "bf/oa/streams/refact." To review the changes, refer to the pull request "OA: Refactoring Streams #1032." This includes updates to the Deriver class, its related documentation, and its class diagram. If you have time, please review the changes and provide feedback on any areas that may need improvement. Thank you!

Hi @steveyuwono, I'll take a look asap and let you know.

@steveyuwono steveyuwono linked a pull request Jul 19, 2024 that will close this issue
steveyuwono added a commit that referenced this issue Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BF Basic Functions/Infrastructure bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants