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

Various minor fixes #258

Merged
merged 7 commits into from
Apr 15, 2024
Merged

Various minor fixes #258

merged 7 commits into from
Apr 15, 2024

Conversation

bockthom
Copy link
Collaborator

@bockthom bockthom commented Apr 12, 2024

Prerequisites

  • I adhere to the coding conventions (described here) in my code.
  • I have updated the copyright headers of the files I have modified.
  • I have written appropriate commit messages, i.e., I have recorded the goal, the need, the needed changes, and the location of my code modifications for each commit. This includes also, e.g., referencing to relevant issues.
  • I have put signed-off tags in all commits.
  • I have updated the changelog file NEWS.md appropriately.
  • I have checked whether I need to adjust the showcase file showcase.R with respect to my changes.
  • The pull request is opened against the branch dev.

Description

This PR contains a few minor fixes in various parts of coronet (i.e., add some checks for corner cases) and restores the intended behavior of get.expanded.adjacency.matrices, which has been broken recently.

Changelog

…ed.by.timestamps`

If no custom event timestamps are available in the ProjectData object, an
error is thrown in `split.data.time.based.by.timestamps`, to avoid passing
`NULL` to the called splitting functions.

Signed-off-by: Thomas Bock <[email protected]>
To avoid running into various errors when calculating scale-freeness
for an empty network, check if the network is empty before calculating
scale-freeness and return `NA` right away if the network is empty.

Signed-off-by: Thomas Bock <[email protected]>
In commit e72eff8, the function
`get.expanded.adjacency.matrices` has been refactored. Unfortunately,
the author names have only be extracted from each network individually
instead of globally over all networks. This way, the functionality of
the function has been changed inadvertently. With this commit, we
follow-up on this and restore the previous behavior.

As `get.author.names.from.networks(networks)` now always returns a list,
accessing the first element is necessary (although this was not
necessary in previous implementations).

Moreover, this commit also fixes a wrong test with respect to
get.expanded.adjacency.matrices`.

Signed-off-by: Thomas Bock <[email protected]>
To stay consistent with the naming scheme of our test files,
rename 'test-util-networks-misc.R' to 'test-networks-misc.R'.

Signed-off-by: Thomas Bock <[email protected]>
In various of our source-code files, our coding conventions
have been violated. With this commit, we fix the most obvious violations:

- semicolon at the end of a statement
- missing space between "if" and the subsequent "("
- superfluous space between "return" and "("
- inconsistent indentation

Signed-off-by: Thomas Bock <[email protected]>
@bockthom bockthom added this to the v4.4 milestone Apr 12, 2024
@bockthom bockthom requested a review from hechtlC April 12, 2024 16:12
The bug in igraph's `disjoint_union` regarding unintended type
conversions is still present in the current igraph version 2.0.3.
Therefore, we update the corresponding comment in our source code.

Signed-off-by: Thomas Bock <[email protected]>
Signed-off-by: Thomas Bock <[email protected]>
Copy link
Contributor

@hechtlC hechtlC left a comment

Choose a reason for hiding this comment

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

Looks good thanks @bockthom !
I will merge this.

@hechtlC hechtlC merged commit 2bd6132 into se-sic:dev Apr 15, 2024
6 checks passed
@bockthom bockthom mentioned this pull request Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants