Ndr/cleanup #30
Ndr/cleanup #30
Conversation
|
Just added in some updates to the docs to point to the conda-forge package for an installation option. |
There was a problem hiding this comment.
some nice and sensible changes in here. i'm sure that we just took care of some of issue #2 in the process.
a silly find-and-replace text fix requested.
also, can you explain to me how we can load nrel.routee.compass now without init.py files? shouldn't it just import as import compass now? or does the directory position relative to the pyproject.toml file somehow implicitly form package names?
oh, and, why the name change to graph_struct for the graph module?
| let pos: Option<Vec<usize>> = Some(vec![0 as usize; sets.len()]); | ||
| return MultiSet { | ||
| let final_pos: Vec<usize> = sets.iter().map(|v| v.len() - 1).collect(); | ||
| let pos: Option<Vec<usize>> = Some(vec![0_usize; sets.len()]); |
There was a problem hiding this comment.
this looks like a typo. is 0_usize valid rust code?
There was a problem hiding this comment.
This was one of the auto clippy fixes (here's the lint doc) and I didn't know this but apparently it is valid rust code. But just putting in 0 would also work since we have an explicit type on the left and so maybe I just do that
| @@ -7,9 +7,9 @@ pub trait CompassJsonExtensions { | |||
|
|
|||
| impl CompassJsonExtensions for serde_json::Value { | |||
| /// attempts to read queries from the user in the three following ways: | |||
| /// 1. top-level of JSON is an array -> return it directly | |||
There was a problem hiding this comment.
victims of find-and-replace on the word "return", can you return these to how they were?
There was a problem hiding this comment.
whoops! yes good catch
The name change was the the result of this clippy suggestion: https://rust-lang.github.io/rust-clippy/master/index.html#/module_inception
This was the result of PEP 420 that I only recently learned about. Now, namespaces are preferred to be implicit rather than explicit (as they used to be here) and python can understand how to get to the base level package. This became important when I attempted to build a new namespace package that depended on these other namespace packages. |
There was a problem hiding this comment.
thanks for all of the improvements here! i do think "graph_struct" is weird, maybe what we need is to change the outer module name to "road_network" or something like that, and keep the inner module name "graph"? but i'll leave that to you if you like it, want to leave it or have a different idea.
yeah I like that idea, just pushed up that change |
Address a couple of items:
cargo clippyand and fix all the errors and warnings that came upI also updated VSCode to use clippy to check and lint the code instead of the default of
cargo check. I had to add the following line to mysettings.jsonfile: