Merge pull request #47 from graspologic-org/dev

1.2.2 Release

Author: daxpryce

.github/workflows/build.yml

@@ -12,10 +12,10 @@ jobs:
     runs-on: "ubuntu-latest"
     steps:
       - uses: actions/checkout@v4
-      - name: Set up Python 3.8
+      - name: Set up Python 3.12
         uses: actions/setup-python@v5
         with:
-          python-version: 3.8
+          python-version: 3.12
       - name: Materialize build number
         run: |
           pip install -U pip
@@ -38,10 +38,10 @@ jobs:
         os: ["ubuntu-latest", "windows-latest", "macos-latest"]
     steps:
       - uses: actions/checkout@v4
-      - name: Set up Python 3.9
+      - name: Set up Python 3.12
         uses: actions/setup-python@v5
         with:
-          python-version: 3.9
+          python-version: 3.12
       - uses: actions/download-artifact@v4
         with:
           name: cargo-toml
@@ -106,10 +106,10 @@ jobs:
     if: github.ref=='refs/heads/main' || github.ref=='refs/heads/dev'
     steps:
       - uses: actions/checkout@v4
-      - name: Set up Python 3.8
+      - name: Set up Python 3.12
         uses: actions/setup-python@v2
         with:
-          python-version: 3.8
+          python-version: 3.12
       - uses: actions/download-artifact@v4
         with:
           name: dist-ubuntu-latest

.rustfmt.toml

@@ -1,3 +1,2 @@
-fn_args_layout = "vertical"
-empty_item_single_line = false
+fn_params_layout = "vertical"
 

clippy.toml

@@ -0,0 +1,2 @@
+too-many-arguments-threshold=20 # for what it is worth, clippy is absolutely right and pythonic-ness is absolutely wrong
+enum-variant-name-threshold=10 # it doesn't like the repetition in "Error" in my export-to-python error types

.cargo/config config.toml.cargo/config renamed to config.toml

@@ -1,16 +1,13 @@
 [package]
 name = "cli"
-version = "0.1.0"
-authors = ["Dwayne Pryce <dwpryce@microsoft.com>"]
+version = "0.1.1"
+authors = ["Dax Pryce <daxpryce@microsoft.com>"]
 edition = "2018"
 license = "MIT"
 description = "CLI Runner for the topologic associated crates (network_partitions and eventually network_automatic_layouts)"
 
 [dependencies]
-clap = "2.34"
+clap = "4.5"
 rand = "0.8"
 rand_xorshift = "0.3"
 network_partitions={path = "../network_partitions"}
-
-[features]
-logging = ["network_partitions/logging"]

packages/cli/Cargo.toml

@@ -33,27 +33,30 @@ pub struct CliArgs {
     pub skip_first_line: bool,
 }
 
-impl TryFrom<ArgMatches<'_>> for CliArgs {
+impl TryFrom<ArgMatches> for CliArgs {
     type Error = ParseCliError;
 
-    fn try_from(matches: ArgMatches<'_>) -> Result<Self, Self::Error> {
-        let source_edges = matches
-            .value_of(SOURCE_EDGES)
+    fn try_from(matches: ArgMatches) -> Result<Self, Self::Error> {
+        let source_edges: &str = matches
+            .get_one(SOURCE_EDGES)
+            .cloned()
             .ok_or(ParseCliError::RequiredValueError)?;
-        let output = matches
-            .value_of(OUTPUT)
+        let output: &str = matches
+            .get_one(OUTPUT)
+            .cloned()
             .ok_or(ParseCliError::RequiredValueError)?;
-        let separator = matches
-            .value_of(SEPARATOR)
+        let separator: &str = matches
+            .get_one(SEPARATOR)
+            .cloned()
             .ok_or(ParseCliError::RequiredValueError)?;
-        let source_index: usize = matches.value_of(SOURCE_INDEX).as_a()?;
-        let target_index: usize = matches.value_of(TARGET_INDEX).as_a()?;
-        let weight_index: Option<usize> = matches.value_of(WEIGHT_INDEX).as_a()?;
-        let seed: Option<usize> = matches.value_of(SEED).as_a()?;
-        let iterations: usize = matches.value_of(ITERATIONS).as_a()?;
-        let resolution: f64 = matches.value_of(RESOLUTION).as_a()?;
-        let randomness: f64 = matches.value_of(RANDOMNESS).as_a()?;
-        let quality_option: Option<&str> = matches.value_of(QUALITY);
+        let source_index: usize = *matches.get_one(SOURCE_INDEX).unwrap();
+        let target_index: usize = *matches.get_one(TARGET_INDEX).unwrap();
+        let weight_index: Option<usize> = matches.get_one(WEIGHT_INDEX).copied();
+        let seed: Option<usize> = matches.get_one(SEED).cloned();
+        let iterations: usize = *matches.get_one(ITERATIONS).unwrap();
+        let resolution: f64 = *matches.get_one(RESOLUTION).unwrap();
+        let randomness: f64 = *matches.get_one(RANDOMNESS).unwrap();
+        let quality_option: Option<&str> = matches.get_one(QUALITY).cloned();
         let use_modularity: bool = match quality_option {
             Some(quality_value) => {
                 if quality_value == "cpm" {
@@ -66,7 +69,7 @@ impl TryFrom<ArgMatches<'_>> for CliArgs {
             }
             None => Err(ParseCliError::RequiredValueError),
         }?;
-        let skip_first_line: bool = matches.is_present(HAS_HEADER);
+        let skip_first_line: bool = matches.contains_id(HAS_HEADER);
         let cli_args: CliArgs = CliArgs {
             source_edges: source_edges.into(),
             output_path: output.into(),
@@ -81,7 +84,7 @@ impl TryFrom<ArgMatches<'_>> for CliArgs {
             use_modularity,
             skip_first_line,
         };
-        return Ok(cli_args);
+        Ok(cli_args)
     }
 }
 
@@ -94,43 +97,12 @@ pub enum ParseCliError {
 
 impl From<ParseFloatError> for ParseCliError {
     fn from(_: ParseFloatError) -> Self {
-        return ParseCliError::NotANumber;
+        ParseCliError::NotANumber
     }
 }
 
 impl From<ParseIntError> for ParseCliError {
     fn from(_: ParseIntError) -> Self {
-        return ParseCliError::NotANumber;
-    }
-}
-
-trait As<T> {
-    fn as_a(&self) -> Result<T, ParseCliError>;
-}
-
-impl As<f64> for Option<&str> {
-    fn as_a(&self) -> Result<f64, ParseCliError> {
-        self.map(|cli_arg| cli_arg.parse::<f64>().unwrap())
-            .ok_or(ParseCliError::RequiredValueError)
-    }
-}
-
-impl As<usize> for Option<&str> {
-    fn as_a(&self) -> Result<usize, ParseCliError> {
-        self.map(|cli_arg| cli_arg.parse::<usize>().unwrap())
-            .ok_or(ParseCliError::RequiredValueError)
-    }
-}
-
-impl As<Option<usize>> for Option<&str> {
-    fn as_a(&self) -> Result<Option<usize>, ParseCliError> {
-        let result = match self {
-            Some(cli_arg) => {
-                let parse_result = cli_arg.parse::<usize>();
-                Ok(parse_result.map(|value| Some(value))?)
-            }
-            None => Ok(None),
-        };
-        return result;
+        ParseCliError::NotANumber
     }
 }

packages/cli/src/args.rs

@@ -75,9 +75,9 @@ pub fn leiden(
             let mut output_file: File =
                 File::create(output_path).expect("Unable to open output file for writing");
             for item in &clustering {
-                write!(
+                writeln!(
                     output_file,
-                    "{},{}\n",
+                    "{},{}",
                     labeled_network.label_for(item.node_id),
                     item.cluster
                 )

packages/cli/src/leiden.rs

@@ -1,8 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-#![feature(in_band_lifetimes)]
-use clap::{App, Arg};
+use clap::{Arg, ArgAction, Command};
 use std::convert::TryFrom;
 
 mod args;
@@ -11,83 +10,82 @@ mod leiden;
 use crate::args::*;
 
 fn main() {
-    let matches = App::new("leiden_cli")
+    let matches = Command::new("leiden_cli")
         .version("0.1.0")
         .author("Dwayne Pryce <[email protected]>")
         .about("Runs leiden over a provided edge list and outputs the results")
         .arg(
-            Arg::with_name(SOURCE_EDGES)
+            Arg::new(SOURCE_EDGES)
                 .help("The edge list that defines the graph's connections")
                 .required(true)
                 .index(1),
         )
         .arg(
-            Arg::with_name(OUTPUT)
+            Arg::new(OUTPUT)
                 .help("The output for the communities detected")
                 .required(true)
                 .index(2),
         )
         .arg(
-            Arg::with_name(SEPARATOR)
-                .short("s")
+            Arg::new(SEPARATOR)
+                .short('s')
                 .help("The character to split the edge list on")
-                .takes_value(true)
+                .action(ArgAction::Set)
                 .default_value("\t"),
         )
         .arg(
-            Arg::with_name(SOURCE_INDEX)
-                .takes_value(true)
+            Arg::new(SOURCE_INDEX)
+                .action(ArgAction::Set)
                 .help("0-based index of source column from edge file")
                 .default_value("0"),
         )
         .arg(
-            Arg::with_name(TARGET_INDEX)
-                .takes_value(true)
+            Arg::new(TARGET_INDEX)
+                .action(ArgAction::Set)
                 .help("0-based index of target column from edge file")
                 .default_value("1"),
         )
         .arg(
-            Arg::with_name(WEIGHT_INDEX)
-                .takes_value(true)
+            Arg::new(WEIGHT_INDEX)
+                .action(ArgAction::Set)
                 .help("0-based index of weight column from edge file")
         )
         .arg(
-            Arg::with_name(SEED)
-                .takes_value(true)
+            Arg::new(SEED)
+                .action(ArgAction::Set)
                 .help("A seed value to start the PRNG")
                 .long("seed"),
         )
         .arg(
-            Arg::with_name(ITERATIONS)
-                .takes_value(true)
+            Arg::new(ITERATIONS)
+                .action(ArgAction::Set)
                 .help("Leiden is an inherently recursive algorithm, however it may find itself (due to randomness) at a localized maximum. Setting iterations to a number larger than 1 may allow you to jump out of a local maximum and continue until a better optimum partitioning is found (note that any n > 1 will mean that leiden will be run again for a minimum of n-1 more times, though it may be run for many more than that")
-                .short("i")
+                .short('i')
                 .default_value("1"),
         )
         .arg(
-            Arg::with_name(RESOLUTION)
-                .takes_value(true)
+            Arg::new(RESOLUTION)
+                .action(ArgAction::Set)
                 .help("")
-                .short("r")
+                .short('r')
                 .default_value("1.0")
         )
         .arg(
-            Arg::with_name(RANDOMNESS)
-                .takes_value(true)
+            Arg::new(RANDOMNESS)
+                .action(ArgAction::Set)
                 .help("")
                 .default_value("1E-2"),
         )
         .arg(
-            Arg::with_name(QUALITY)
-                .takes_value(true)
+            Arg::new(QUALITY)
+                .action(ArgAction::Set)
                 .help("Quality function to use")
-                .short("q")
-                .possible_value("modularity")
-                .possible_value("cpm")
+                .short('q')
+                .value_parser(["modularity", "cpm"])
                 .default_value("modularity"),
         )
         .arg(
-            Arg::with_name(HAS_HEADER)
+            Arg::new(HAS_HEADER)
                 .help("Flag must be added if the source file contains a header line")
                 .long("has_header")
         )

packages/cli/src/main.rs

@@ -1,18 +1,14 @@
 [package]
 name = "network_partitions"
 version = "0.1.0"
-authors = ["Dwayne Pryce <dwpryce@microsoft.com>"]
+authors = ["Dax Pryce <daxpryce@microsoft.com>"]
 edition = "2018"
 license = "MIT"
 description = "Leiden community detection as per https://arxiv.org/abs/1810.08473"
 
 [dependencies]
 rand = "0.8"
-chrono = { version = "0.4", optional = true }
 
 [dev-dependencies]
 rand_xorshift = "0.3"
 
-[features]
-logging = ["chrono"]
-debug = []