-
Notifications
You must be signed in to change notification settings - Fork 54
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 ability to add features to existing track without retracking #461
base: RC_v1.5.x
Are you sure you want to change the base?
Add ability to add features to existing track without retracking #461
Conversation
…not working, unfortunately.
…d to do some tests with real data, though.
# Conflicts: # tobac/tracking.py
I should have noted that I put in a good bit of effort to refactor the primary tracking function to make it less of a monolith. I'm not sure I succeeded there, but it is a bit cleaner now. |
Linting results by Pylint:Your code has been rated at 8.74/10 (previous run: 8.74/10, +0.00) |
Nice work! I’ll try and start reviewing this next week. Does this include variable dt capability as well? |
Inherently, yes it does. It would be good to add a wrapper that automagically does this, though. Would be good to get as part of 1.6 (or perhaps a 1.5.5). A nice feature for folks. |
okay I'm looking into the codecov failure. It seems to be a real failure, but only when running coverage, which is just frankly bizarre. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## RC_v1.5.x #461 +/- ##
=============================================
+ Coverage 60.72% 61.80% +1.07%
=============================================
Files 23 23
Lines 3537 3744 +207
=============================================
+ Hits 2148 2314 +166
- Misses 1389 1430 +41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This seems to be a problem with |
something is wrong with the linting CI; GitHub isn't pulling the newest version. |
Ran into this bug while testing:
I'll have a look deeper into what is going on in the morning |
Ok, so the issue I'm encountering is that on line 820, the result of
is 2 for every row. I'll have to dig a bit deeper into why this is occurring. Do you have an example of applying the append_tracks method to real data so I can see if I'm using it correctly? |
tracks_orig_retrack = copy.deepcopy( | ||
tracks_orig[tracks_orig["frame"].isin(frames_orig_cut)] | ||
) | ||
tracks_orig_retrack["cell"].replace(cell_number_unassigned, np.nan, inplace=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line produces the following warning, which will become an exception in future versions of pandas:
FutureWarning: A value is trying to be set on a copy of a DataFrame or Series through chained assignment using an inplace method.
The behavior will change in pandas 3.0. This inplace method will never work because the intermediate object on which we are setting values always behaves as a copy.
For example, when doing 'df[col].method(value, inplace=True)', try using 'df.method({col: value}, inplace=True)' or df[col] = df[col].method(value) instead, to perform the operation inplace on the original object.
It also converts the type of the cell column to floating, which I don't think is desirable behaviour. Is there a way of avoiding this?
This new function (and its various support functions) is designed to allow the user to append a newly detected feature DataFrame to an existing set of tracks. This (should) work with all existing tobac tracking options, except memory due to an upstream issue I've documented here: soft-matter/trackpy#776 .
I have added docs, but haven't yet added a notebook. I'd like to do this before this function is merged, but wanted to start the discussion now.