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

Missing range names when splitting networks time-based #273

Open
bockthom opened this issue Nov 2, 2024 · 0 comments
Open

Missing range names when splitting networks time-based #273

bockthom opened this issue Nov 2, 2024 · 0 comments
Assignees
Milestone

Comments

@bockthom
Copy link
Collaborator

bockthom commented Nov 2, 2024

Description

The function split.networks.time.based (i.e., plural version networks) has inconsistent behavior regarding range names:

If sliding.window = FALSE, the returned list of networks contains range names as the names of the list.

However, if sliding.window = TRUE, there are no names attached to the returned list.

This difference might be caused by the two different function calls here (within split.networks.time.based) :

coronet/util-split.R

Lines 644 to 650 in 751b72f

if (sliding.window) {
nets = split.network.time.based.by.ranges(network = net, ranges = ranges,
remove.isolates = remove.isolates)
} else {
nets = split.network.time.based(network = net, bins = bins.date, sliding.window = sliding.window,
remove.isolates = remove.isolates)
}

While the output of split.network.time.based seems to contain the names, the output of split.network.time.based.by.ranges does not contain the names. However, I am not sure where we actually loose the range names –– split.network.time.based.by.ranges internally calls split.network.time.based.

Versions

At least, the most recent version 4.4 of coronet is affected, and also the current dev branch still contains this inconsistency. I did not check whether we have introduced this inconsistency in version 4.4 or whether this has already been present from the introduction of function split.networks.time.based on.

@bockthom bockthom added this to the v4.5 milestone Nov 2, 2024
maxloeffler added a commit to maxloeffler/coronet that referenced this issue Nov 12, 2024
This change makes splitting by ranges more consistent with time-based
splitting.

This works towards se-sic#273.

Signed-off-by: Maximilian Löffler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants