Add obj weights to Aggregator (resolves #368)

[Naarlack]

Added a sequences_obj_log dict to keep track of the unique trajectory objects that are detected in each flow. For each trajectory that is processed, the .obj_id is added to a set at each corresponding sequence key. The length of the set is then used to generate an obj_weight in the flow lines output.

UNRELATED: I saw that the .get_xxx functions all had a check for whether the relevant data was available and, if not, the relevant update function was being called. However, the function result was never assigned.

I was also very confused by the hour values being inserted into self.id_to_centroid. I'm not sure why that's being done?

PS. This is my first ever pull request so please let me know whether I'm doing anything wrong 😅

[Naarlack]

As noted, this is my first pull request. I was wondering: should I have created a new branch prior to submission?

Also, I've realised I did not make any corresponding updates to class descriptions or documentation. Would it be typical for a pull request to include those, or should that happen as a follow up after the commits are approved?

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 90.47619% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 95.73%. Comparing base (95d7098) to head (d04b725).

Files	Patch %	Lines
movingpandas/trajectory_aggregator.py	66.66%	3 Missing ⚠️
movingpandas/tests/test_trajectory_aggregator.py	96.96%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #369   +/-   ##
=======================================
  Coverage   95.73%   95.73%           
=======================================
  Files          33       33           
  Lines        3748     3777   +29     
=======================================
+ Hits         3588     3616   +28     
- Misses        160      161    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[anitagraser]

Please include the updates to the doctoring as well. We'll want to merge the whole package.

We'll also need a new unit test that ensures that the code is working as expected (and that it keeps working in case we do refactoring).

[Naarlack]

OK, I think I have done both those things.

I'm assuming the docs get updated from the method docstrings.

I have not worked with Pytest much, but I edited the current flow test to include the new obj_weight and it's passing so I hope that means it works.

[anitagraser]

Please add a new test instead of changing the existing one because we want to ensure that it works with and without these weights

[anitagraser]

Ah. I see that obj_weight is now always included.

[Naarlack]

Yes, is that OK?

It seemed logical to me.

I guess the only issue would be if the trajectories did not have an obj_id (i.e. None), in which case the obj_weight would just be 1 for each flow segment. But that seems like a reasonable output for that particular situation I think.

[anitagraser]

The safest thing would probably be to have two separate tests, one with object ids and one without.

[Naarlack]

I think I have done what you recommended, but I'm not great with Pytest. You might need to go in and adjust yourself if it's not what you had in mind.

[anitagraser]

Thank you, @Naarlack. I've split up the tests further to explicitly test for the correct behavior both with and without obj_id_col in the TrajectoryCollection constructor. All looks good now. Thank you for this contribution.

[raybellwaves]

Thanks @Naarlack

[Naarlack]

Happy to be involved @anitagraser, @raybellwaves - thanks for putting up with all my questions 😅

I think this is a really useful library so hopefully I can contribute more.

If you're interested, this is my project that got me into processing moving data: https://project.velograph.app/