Preparing for 1.2.2 and updating versions #46

daxpryce · 2025-01-07T21:17:32Z

This PR got away from me for sure. I updated versions, I fixed all the breaks that entailed, I ran clippy for apparently the first time, I fixed all of the warnings (well, almost all - see clippy.toml for details), I ran cargo fmt, I gave up on arguing about:

struct Foo { }
// vs.
struct Bar {
}

Because arguing with my formatting tools is lame and a gigantic waste of time.

I also added some dev deps to make it easy to get into a repl with it in by bootstrapping via uv. And I found out my unit tests only get run if I do it manually in dax space, so like.. we probably should write some actual damn tests into the CICD for this. I pulled the plug on doing that right now because I don't want to throw a thousand changes at you.

And I didn't change the API, so this is just a patch, though I changed enough of the internals I'm a bit nervous about saying that for my downstream users. Especially without any meaningful and exhaustive tests. 🙀

daxpryce · 2025-01-08T02:43:35Z

Some of the CommonMark doc changes clippy made for me ended up being wrong, so I'll be going through the API docs with a finetooth comb tomorrow morning for sure

darthtrevino

Just a few small clarifying questions, but otherwise it looks pretty straightforward

darthtrevino · 2025-01-08T17:27:29Z

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


Can you clarify what you mean here? Is this an issue with named kwargs or positional args?

the number of arguments. by default, clippy will warn you that your callable signature is too long if you have more than 7 arguments.

I've certainly seen **kwargs used for this, but it's not universal. and even if it was, it isn't worth me changing the api

darthtrevino · 2025-01-08T17:32:36Z

packages/network_partitions/src/leiden/neighboring_clusters.rs

-                    self.neighbor_edge_weights_within_cluster[*cluster] = f64::NAN;
-                }
-                self.neighboring_clusters.clear();
+        if let Some(current_cluster) = self.current_cluster {


It's been a minute since I've written any Rust, but this looks like an assignment in an if expression, and I just wanted to confirm your intent here.

I believe it is - I think self.current_cluster can be None, and rather than try to match on a Some(current_cluster) and handle the None in said match when I don't actually want to do anything if it is None, I fabricate a Some(usize) and test equality. I don't know why I thought this was strictly better than the other way, it has been a few years at this point. Currently I have zero strong opinions about it

darthtrevino · 2025-01-08T17:34:15Z

packages/pyo3/pyproject.toml

+[tool.uv]
+dev-dependencies = [
+    "ipython>=8.12.3",
+    "networkx==3",


Will this pin to 3.0.0 or will it allow minor/patch bumps?

I'm not sure, so I'll just be explicit and use >=3,<4

Expanding our supported python trove identifiers to include python 3.13

Updating pyo3 from 0.15 to 0.23

Moving 3.6 and 3.7 to EOL (since not only they, but even 3.8 are EOL. pyo3 currently does not support abi3-py36, and I'm betting 3.7 is going to follow suit sometime soon.)

abi3-py36 changing to abi3-py38

…nature specification for python. Also updated my first name everywhere.

…was never used and never should be used (there have to be better ways than that nonsense), and about to have clippy fix my convention of always using a return statement because I hate implicit returns but that's a me thing not a world thing

… && uv run ipython` get you to a reasonable repl for manual testing. I cannot believe I did not do proper python testing here. Maybe I did it in graspologic?

…mmit message alone

…v/main branching scheme.

…cification. I really hope this doesn't break older versions.

…n in the function. Too much was being treated as a quoted paragraph.

daxpryce · 2025-01-08T21:53:00Z

force push is due to not merging the prior changes from @dylanwhawk to dev and rebasing on that.

daxpryce requested a review from darthtrevino January 7, 2025 21:17

daxpryce changed the title ~~Update build.yml~~ Preparing for 1.2.2 and updating versions Jan 8, 2025

daxpryce marked this pull request as ready for review January 8, 2025 02:28

daxpryce mentioned this pull request Jan 8, 2025

Update pyproject for maturin 1.8.0 #45

Merged

bryantower approved these changes Jan 8, 2025

View reviewed changes

darthtrevino previously approved these changes Jan 8, 2025

View reviewed changes

daxpryce added 15 commits January 8, 2025 20:57

Update build.yml

a1622d1

Update pyproject.toml

44b227b

Expanding our supported python trove identifiers to include python 3.13

Update Cargo.toml

743dbe8

Updating pyo3 from 0.15 to 0.23

Update pyproject.toml

7694603

Moving 3.6 and 3.7 to EOL (since not only they, but even 3.8 are EOL. pyo3 currently does not support abi3-py36, and I'm betting 3.7 is going to follow suit sometime soon.)

Update Cargo.toml

2421ed8

abi3-py36 changing to abi3-py38

Updated a bunch of versions. Pyo3 changed fairly substantially in sig…

998929d

…nature specification for python. Also updated my first name everywhere.

More clippy fixes

4ef2381

More clippy suggestions. It's like I never ran this before.

5afb85b

Adding ipython and networkx as dev deps. `cd packages/pyo3 && uv sync…

1135e5e

… && uv run ipython` get you to a reasonable repl for manual testing. I cannot believe I did not do proper python testing here. Maybe I did it in graspologic?

Running cargo fmt

a765c9c

Misspelled repetition, which I have also repetitively done in this co…

4d4b70c

…mmit message alone

Committing some minor changes before I rebase on dev. I forgot the de…

1cc658f

…v/main branching scheme.

Updating the pyproject.toml to be correct as per the current pypa spe…

775e22e

…cification. I really hope this doesn't break older versions.

Fixing the changes clippy made to some of the commonmark documentatio…

f3994bd

…n in the function. Too much was being treated as a quoted paragraph.

daxpryce dismissed darthtrevino’s stale review via f3994bd January 8, 2025 21:52

daxpryce force-pushed the prep-1.2.2 branch from 073f1ab to f3994bd Compare January 8, 2025 21:52

daxpryce requested review from bryantower and darthtrevino January 8, 2025 21:55

bryantower approved these changes Jan 8, 2025

View reviewed changes

daxpryce merged commit 0ea1c92 into dev Jan 8, 2025
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preparing for 1.2.2 and updating versions #46

Preparing for 1.2.2 and updating versions #46

daxpryce commented Jan 7, 2025 •

edited

Loading

daxpryce commented Jan 8, 2025

darthtrevino left a comment

darthtrevino Jan 8, 2025

daxpryce Jan 8, 2025

darthtrevino Jan 8, 2025

daxpryce Jan 8, 2025

darthtrevino Jan 8, 2025

daxpryce Jan 8, 2025

daxpryce commented Jan 8, 2025

		@@ -0,0 +1,2 @@
		too-many-arguments-threshold=20 # for what it is worth, clippy is absolutely right and pythonic-ness is absolutely wrong

Preparing for 1.2.2 and updating versions #46

Preparing for 1.2.2 and updating versions #46

Conversation

daxpryce commented Jan 7, 2025 • edited Loading

daxpryce commented Jan 8, 2025

darthtrevino left a comment

Choose a reason for hiding this comment

darthtrevino Jan 8, 2025

Choose a reason for hiding this comment

daxpryce Jan 8, 2025

Choose a reason for hiding this comment

darthtrevino Jan 8, 2025

Choose a reason for hiding this comment

daxpryce Jan 8, 2025

Choose a reason for hiding this comment

darthtrevino Jan 8, 2025

Choose a reason for hiding this comment

daxpryce Jan 8, 2025

Choose a reason for hiding this comment

daxpryce commented Jan 8, 2025

daxpryce commented Jan 7, 2025 •

edited

Loading