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

Define split_datasets in data.tracking module instead of data package #642

Open
rossbar opened this issue Jan 11, 2023 · 2 comments
Open
Labels
chore Maintenance

Comments

@rossbar
Copy link
Contributor

rossbar commented Jan 11, 2023

The split_datasets function is only used internally in the data.tracking module - it would clean things up a bit to define the function there rather than at the package level (i.e. in data/__init__.py).

I think this should be possible without affecting the user API, so hopefully no deprecations etc. Opening here in case there are any objections or I've missed something obvious!

@rossbar rossbar added the chore Maintenance label Jan 11, 2023
@msschwartz21
Copy link
Member

I think there was originally an intention to implement other datasets in which case the split_datasets function would be used in other modules. However I don't think that we are even using the prepare_dataset function that calls split_datasets in the tracking module. We have moved towards defining stable dataset splits in the data registry as opposed to splitting the data right before training.

I'm fine with moving split_datasets into data.tracking, but I think we should make a note that some of this code is no longer in use.

@rossbar
Copy link
Contributor Author

rossbar commented Jan 11, 2023

Yeah there's certainly a larger discussion to be had about which data loading/generating functionality in deepcell-tf is still in use with the current pipelines, and what (if any) may be candidates for deprecation. Thanks for the feedback!

rossbar added a commit to rossbar/deepcell-tf that referenced this issue Jan 11, 2023
Fixing would introduce circular import - see vanvalenlabgh-642.
rossbar added a commit to rossbar/deepcell-tf that referenced this issue Jan 13, 2023
Fixing would introduce circular import - see vanvalenlabgh-642.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Maintenance
Projects
None yet
Development

No branches or pull requests

2 participants