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

Add new QL endpoints of /v2/attr and /v2/attr/{attr_name} #16

Merged
merged 440 commits into from
Jun 18, 2024

Conversation

tzuchen-liu
Copy link
Contributor

No description provided.

@tzuchen-liu tzuchen-liu requested a review from tstorek August 29, 2021 05:51
tstorek
tstorek previously requested changes Sep 9, 2021
Copy link
Collaborator

@tstorek tstorek left a comment

Choose a reason for hiding this comment

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

@orange95 thanks, for this pull request. Unfortunatly, there are no tests provided for the functionality. Please, do so. Additionally, it would great if you state why you deviate from the TimeSeries pattern. I think, that all queries that actually contain values should be converted to Either TimeSeries objects or to list of TimeSeriesObjects

entityType: str = Field(default=None,
alias="entityType",
description="The type of an entity")
entities: List[Any] = Field(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aren't the entities a list of something? I think here the same applies

title="Attribute name",
description=""
)
types: List[Any] = Field(
Copy link
Collaborator

Choose a reason for hiding this comment

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

according to the docs there is model used for the types. .. Please reconsider how you use the models. The field type containes a list of entities and the a entityType

{
"application/json": {
"attrs:": [
{
"attrName": "temperature",
"types": [
{
"entities": [
{
"entityId": "Room1",
"index": [
"2018-01-05T15:44:34",
"2018-01-06T15:44:59",
"2018-01-07T15:44:59"
],
"values": [
24.1,
25.3,
26.7
]
},
{
"entityId": "Room2",
"index": [
"2018-01-05T15:44:34",
"2018-01-06T15:44:59",
"2018-01-07T15:44:59"
],
"values": [
22.1,
23.3,
25.7
]
}
],
"entityType": "Room"
},
{
"entities": [
{
"entityId": "DeviceInRoom1",
"index": [
"2018-01-05T15:44:34",
"2018-01-06T15:44:59",
"2018-01-07T15:44:59"
],
"values": [
24.1,
25.3,
26.7
]
},
{
"entityId": "DeviceInRoom2",
"index": [
"2018-01-05T15:44:34",
"2018-01-06T15:44:59",
"2018-01-07T15:44:59"
],
"values": [
22.1,
23.3,
25.7
]
}
],
"entityType": "Device"
}
]
}
]
}
}

offset=offset)
return [TimeSeriesEntities(
entityType=entities_item.get('entityType'),
entities=[EntityValues(
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a possibility to use itertools.product here instead of nested for-loops?

to_date=to_date,
limit=limit,
offset=offset)
return [TimeSeriesAttrHeader(
Copy link
Collaborator

Choose a reason for hiding this comment

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

check if you can use itertools instead of nested forloops

to_date=to_date,
limit=limit,
offset=offset)
return [TimeSeriesAttrHeader(
Copy link
Collaborator

Choose a reason for hiding this comment

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

actually everything should somehow a TimeSeries Object in the very end

@tzuchen-liu
Copy link
Contributor Author

as for adding test, I plan to add these to the line 126 of tests/clients/test_ngsi_v2_timeseries.py
now need to pull in order to commit to another issue, so I put the code here

` entity_attr = client.get_entity_by_attrs(
entity_type=self.entity_1.type)
for entity in entity_attr:
print(entity.to_pandas())

        entity_attr_name = client.get_entity_by_attr_name(
            attr_name="temperature", entity_type=self.entity_1.type)
        for entity in entity_attr_name:
            print(entity.to_pandas())

`

@SBlechmann SBlechmann changed the base branch from development to master May 6, 2022 15:12
@tstorek
Copy link
Collaborator

tstorek commented Jul 6, 2022

@SBlechmann is there still any progress on this?

@SBlechmann
Copy link
Contributor

Yes, we are still on it, sorry for the delay.

djs0109 and others added 24 commits December 26, 2023 14:21
…ible-with-Pydantic-v2

fix: updated regex to pattern
# Conflicts:
#	filip/clients/ngsi_v2/cb.py
#	tests/clients/test_ngsi_v2_cb.py
@SystemsPurge
Copy link
Collaborator

@tstorek First , appologies for the long list of commits, only the last three are relevant, the rest stems from a rebase on master because the branch was old. The methods now return the sought after timeseries/timeseries list object. I could not find a way to avoid nested loops, given that the response itself comprises of nested/categorized elements.
One important detail : The get_entity_by_attrs method does not combine the timeseries of same entities for different attributes, since extending the values/index for different attributes doesnt seem to be a trivial task : If done properly, the timestamps in the indexes should be compared one by one, to determine whether adding a None value for a specific attribute is necessary.
Hence why in the test, as u can see, the method now returns a timeseries list of size : number of unique entities x number of unique attributes. The question then becomes, is it ok for this method to return such a result OR, should a timeseries method be implemented, that combines timeseries of same entities with different attributes?

@djs0109 djs0109 self-requested a review May 14, 2024 13:46
geometry=geometry,
coords=coords,
aggr_scope=aggr_scope)
return [TimeSeries(index=item.get('index'),
Copy link
Contributor

Choose a reason for hiding this comment

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

@SystemsPurge could you test here again? It does not work for me. It gives error message:

AttributeError: 'collections.deque' object has no attribute 'get'

Copy link
Collaborator

@SystemsPurge SystemsPurge May 23, 2024

Choose a reason for hiding this comment

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

@djs0109 Could you please give me more information on how to reproduce this ? What versions of the GEs are you running ? Did this error come up in the unittests ?
I've just re-ran the tests ( with added types to account for that scenario ) and it still works for me.
grafik

grafik

@djs0109 djs0109 dismissed tstorek’s stale review June 18, 2024 13:56

already fixed

@djs0109 djs0109 merged commit 0e4290c into master Jun 18, 2024
1 check passed
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.