Skip to content

Commit

Permalink
Force kw-args on more Python functions (#3515)
Browse files Browse the repository at this point in the history
### What
* Part of #3167

This forces the user to write out the names of arguments for a lot more
of our functions. This makes the code easier to read, and more
future-proof for when we add or remove arguments.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3515) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/3515)
- [Docs
preview](https://rerun.io/preview/5d7d9f6bc72e807ea873e42e7e2e1fcf0dfc182d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/5d7d9f6bc72e807ea873e42e7e2e1fcf0dfc182d/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
  • Loading branch information
emilk authored Sep 28, 2023
1 parent b389711 commit a89b6fb
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 70 deletions.
38 changes: 19 additions & 19 deletions crates/re_types_builder/src/codegen/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,8 @@ impl PythonCodeGenerator {
}

// rerun/{datatypes|components|archetypes}/__init__.py
write_init(&kind_path, &mods, files_to_write);
write_init(&test_kind_path, &test_mods, files_to_write);
write_init_file(&kind_path, &mods, files_to_write);
write_init_file(&test_kind_path, &test_mods, files_to_write);
}

fn write_files(&self, files_to_write: &BTreeMap<Utf8PathBuf, String>) {
Expand Down Expand Up @@ -458,7 +458,7 @@ impl PythonCodeGenerator {
}
}

fn write_init(
fn write_init_file(
kind_path: &Utf8PathBuf,
mods: &HashMap<String, Vec<String>>,
files_to_write: &mut BTreeMap<Utf8PathBuf, String>,
Expand Down Expand Up @@ -603,11 +603,7 @@ fn code_for_struct(
2,
4,
);
} else if !obj.is_delegating_component() {
// In absence of a an extension class __init__ method, we don't *need* an __init__ method here.
// But if we don't generate one, LSP will show the class's doc string instead of parameter documentation.
code.push_text(quote_init_method(obj, ext_class, objects), 2, 4);
} else {
} else if obj.is_delegating_component() {
code.push_text(
format!(
"# You can define your own __init__ function as a member of {} in {}",
Expand All @@ -616,6 +612,10 @@ fn code_for_struct(
2,
4,
);
} else {
// In absence of a an extension class __init__ method, we don't *need* an __init__ method here.
// But if we don't generate one, LSP will show the class's doc string instead of parameter documentation.
code.push_text(quote_init_method(obj, ext_class, objects), 2, 4);
}

if obj.is_delegating_component() {
Expand Down Expand Up @@ -1510,7 +1510,7 @@ fn quote_arrow_support_from_obj(
}
}

fn quote_argument_type_alias(
fn quote_parameter_type_alias(
arg_type_fqname: &str,
class_fqname: &str,
objects: &Objects,
Expand All @@ -1535,13 +1535,13 @@ fn quote_argument_type_alias(
}
}

fn quote_init_argument_from_field(
fn quote_init_parameter_from_field(
field: &ObjectField,
objects: &Objects,
current_obj_fqname: &str,
) -> String {
let type_annotation = if let Some(fqname) = field.typ.fqname() {
quote_argument_type_alias(fqname, current_obj_fqname, objects, field.typ.is_plural())
quote_parameter_type_alias(fqname, current_obj_fqname, objects, field.typ.is_plural())
} else {
let type_annotation = quote_field_type_from_field(objects, field, false).0;
// Relax type annotation for numpy arrays.
Expand All @@ -1563,29 +1563,29 @@ fn quote_init_method(obj: &Object, ext_class: &ExtensionClass, objects: &Objects
// If the type is fully transparent (single non-nullable field and not an archetype),
// we have to use the "{obj.name}Like" type directly since the type of the field itself might be too narrow.
// -> Whatever type aliases there are for this type, we need to pick them up.
let arguments: Vec<_> =
let parameters: Vec<_> =
if obj.kind != ObjectKind::Archetype && obj.fields.len() == 1 && !obj.fields[0].is_nullable
{
vec![format!(
"{}: {}",
obj.fields[0].name,
quote_argument_type_alias(&obj.fqname, &obj.fqname, objects, false)
quote_parameter_type_alias(&obj.fqname, &obj.fqname, objects, false)
)]
} else if obj.is_union() {
vec![format!(
"inner: {} | None = None",
quote_argument_type_alias(&obj.fqname, &obj.fqname, objects, false)
quote_parameter_type_alias(&obj.fqname, &obj.fqname, objects, false)
)]
} else {
obj.fields
.iter()
.sorted_by_key(|field| field.is_nullable)
.map(|field| quote_init_argument_from_field(field, objects, &obj.fqname))
.map(|field| quote_init_parameter_from_field(field, objects, &obj.fqname))
.collect()
};
let head = format!("def __init__(self: Any, {}):", arguments.join(", "));
let head = format!("def __init__(self: Any, {}):", parameters.join(", "));

let argument_docs = if obj.is_union() {
let parameter_docs = if obj.is_union() {
Vec::new()
} else {
obj.fields
Expand Down Expand Up @@ -1614,11 +1614,11 @@ fn quote_init_method(obj: &Object, ext_class: &ExtensionClass, objects: &Objects
r#"""""#.to_owned(),
format!("Create a new instance of the {} {doc_typedesc}.", obj.name),
];
if !argument_docs.is_empty() {
if !parameter_docs.is_empty() {
doc_string_lines.push("\n".to_owned());
doc_string_lines.push("Parameters".to_owned());
doc_string_lines.push("----------".to_owned());
for doc in argument_docs {
for doc in parameter_docs {
doc_string_lines.push(doc);
}
};
Expand Down
4 changes: 3 additions & 1 deletion rerun_py/rerun_sdk/rerun/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ def _init_recording_stream() -> None:

def init(
application_id: str,
*,
recording_id: str | None = None,
spawn: bool = False,
init_logging: bool = True,
Expand Down Expand Up @@ -302,7 +303,7 @@ def init(

if init_logging:
new_recording(
application_id,
application_id=application_id,
recording_id=recording_id,
make_default=True,
make_thread_default=False,
Expand All @@ -327,6 +328,7 @@ def init(


def new_recording(
*,
application_id: str,
recording_id: str | None = None,
make_default: bool = False,
Expand Down
2 changes: 2 additions & 0 deletions rerun_py/rerun_sdk/rerun/_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ def _splat() -> cmp.InstanceKeyBatch:
def log(
entity_path: str,
entity: AsComponents | Iterable[ComponentBatchLike],
*,
ext: dict[str, Any] | None = None,
timeless: bool = False,
recording: RecordingStream | None = None,
Expand Down Expand Up @@ -158,6 +159,7 @@ def log(
def log_components(
entity_path: str,
components: Iterable[ComponentBatchLike],
*,
num_instances: int | None = None,
ext: dict[str, Any] | None = None,
timeless: bool = False,
Expand Down
49 changes: 0 additions & 49 deletions rerun_py/rerun_sdk/rerun/_rerun2/__init__.py

This file was deleted.

4 changes: 3 additions & 1 deletion rerun_py/rerun_sdk/rerun/recording.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ def reset_data(self) -> None:
"""Reset the data in the MemoryRecording."""
self.storage.reset_data()

def reset_blueprint(self, add_to_app_default_blueprint: bool = False) -> None:
def reset_blueprint(self, *, add_to_app_default_blueprint: bool = False) -> None:
"""Reset the blueprint in the MemoryRecording."""
self.storage.reset_blueprint(add_to_app_default_blueprint)

def as_html(
self,
*,
width: int = DEFAULT_WIDTH,
height: int = DEFAULT_HEIGHT,
app_url: str | None = None,
Expand Down Expand Up @@ -107,6 +108,7 @@ def as_html(

def show(
self,
*,
other: MemoryRecording | None = None,
width: int = DEFAULT_WIDTH,
height: int = DEFAULT_HEIGHT,
Expand Down

0 comments on commit a89b6fb

Please sign in to comment.