-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat(lib): add scene structures and Sionna compatibility #76
Conversation
import requests | ||
from tqdm import tqdm | ||
|
||
from .. import _core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Two consecutive dots (DOUBLE_PUNCTUATION)
Suggestions: .
, …
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-periods
Rule: https://community.languagetool.org/rule/show/DOUBLE_PUNCTUATION?lang=en-US
Category: PUNCTUATION
def members(tar: tarfile.TarFile): | ||
for member in tar.getmembers(): | ||
if (index := member.path.find("sionna/rt/scenes/")) >= 0: | ||
member.path = member.path[index + 17 :] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Path
, path
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
def members(tar: tarfile.TarFile): | ||
for member in tar.getmembers(): | ||
if (index := member.path.find("sionna/rt/scenes/")) >= 0: | ||
member.path = member.path[index + 17 :] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Path
, path
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
src/scene/sionna.rs
Outdated
#[serde(rename(deserialize = "@type"))] | ||
_type: String, | ||
#[serde(rename(deserialize = "@id"))] | ||
id: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
src/scene/sionna.rs
Outdated
#[serde(rename(deserialize = "@type"))] | ||
_type: String, | ||
#[serde(rename(deserialize = "@id"))] | ||
id: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
Rust benchmark results:
|
Python benchmark results:
|
"download_sionna_scenes", | ||
"get_sionna_scene", | ||
"list_sionna_scenes", | ||
"SionnaScene", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Did you just mean “,” or “)”? (COMMA_PERIOD[1])
Suggestions: ,
, )
Rule: https://community.languagetool.org/rule/show/COMMA_PERIOD?lang=en-US&subId=1
Category: PUNCTUATION
src/scene/sionna.rs
Outdated
#[pyclass(get_all)] | ||
#[derive(Clone, Debug, Deserialize)] | ||
struct SionnaScene { | ||
/// A mapping between material ids and actual |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: IDs
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
src/scene/sionna.rs
Outdated
/// Currently, only BSDF materials are used. | ||
#[serde(rename = "bsdf", deserialize_with = "deserialize_materials")] | ||
materials: HashMap<String, Material>, | ||
/// A mapping between shape ids and actual |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: IDs
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
src/scene/sionna.rs
Outdated
|
||
for material in v { | ||
if let Some(material) = map.insert(material.id.clone(), material) { | ||
log::warn!("duplicate material id, cannot insert '{:?}'", material); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
src/scene/sionna.rs
Outdated
|
||
for shape in v { | ||
if let Some(shape) = map.insert(shape.id.clone(), shape) { | ||
log::warn!("duplicate shape id, cannot insert '{:?}'", shape); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
src/scene/sionna.rs
Outdated
/// E.g., `ply` for Stanford PLY format. | ||
#[serde(rename(deserialize = "@type"))] | ||
r#type: String, | ||
/// The shape id. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
src/scene/sionna.rs
Outdated
/// This path is relative to the scene config file. | ||
#[serde(rename(deserialize = "string"), deserialize_with = "deserialize_file")] | ||
file: String, | ||
/// The material id attached to this object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
The abbreviation for “identification” is spelled all-uppercase, or did you mean “I’d” (= I would/had)? (ID_CASING[1])
Suggestions: I'd
, ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=1
Category: CASING
#[derive(Deserialize)] | ||
struct MaterialId { | ||
#[serde(rename(deserialize = "@id"))] | ||
id: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
format!("{:?}", self) | ||
} | ||
|
||
/// Load a Sionna scene from a XML file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Suggestions: an
URL: https://languagetool.org/insights/post/indefinite-articles/
Rule: https://community.languagetool.org/rule/show/EN_A_VS_AN?lang=en-US
Category: MISC
#[serde(rename(deserialize = "@type"))] | ||
pub(crate) r#type: String, | ||
#[serde(rename(deserialize = "@id"))] | ||
pub(crate) id: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
/// | ||
/// It should be unique (in a given scene). | ||
#[serde(rename(deserialize = "@id"))] | ||
pub(crate) id: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
src/scene/triangle_scene.rs
Outdated
|
||
#[pymethods] | ||
impl TriangleScene { | ||
/// Load a triangle scene from a XML file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Suggestions: an
URL: https://languagetool.org/insights/post/indefinite-articles/
Rule: https://community.languagetool.org/rule/show/EN_A_VS_AN?lang=en-US
Category: MISC
|
||
let mut meshes = HashMap::with_capacity(sionna.shapes.len()); | ||
|
||
for (id, shape) in sionna.shapes.into_iter() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
This abbreviation for “identification” is spelled all-uppercase. (ID_CASING[2])
Suggestions: ID
Rule: https://community.languagetool.org/rule/show/ID_CASING?lang=en-US&subId=2
Category: CASING
"source": [ | ||
"from differt.scene.sionna import get_sionna_scene, download_sionna_scenes\n", | ||
"from differt.scene.triangle_scene import TriangleScene\n", | ||
"from differt.plotting import reuse\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Plotting
, plotting
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
python/differt/plotting/_utils.py
Outdated
|
||
TODO: document this. | ||
""" | ||
old_backend = DEFAULT_BACKEND |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Suggestions: BACKEND
Rule: https://community.languagetool.org/rule/show/ENGLISH_WORD_REPEAT_RULE?lang=en-US
Category: MISC
from beartype import beartype as typechecker | ||
from jaxtyping import Array, Float, jaxtyped | ||
|
||
from .. import _core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Two consecutive dots (DOUBLE_PUNCTUATION)
Suggestions: .
, …
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-periods
Rule: https://community.languagetool.org/rule/show/DOUBLE_PUNCTUATION?lang=en-US
Category: PUNCTUATION
from jaxtyping import Array, Float, jaxtyped | ||
|
||
from .. import _core | ||
from ..geometry.triangle_mesh import TriangleMesh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Two consecutive dots (DOUBLE_PUNCTUATION)
Suggestions: .
, …
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-periods
Rule: https://community.languagetool.org/rule/show/DOUBLE_PUNCTUATION?lang=en-US
Category: PUNCTUATION
|
||
from .. import _core | ||
from ..geometry.triangle_mesh import TriangleMesh | ||
from ..plotting import draw_markers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Two consecutive dots (DOUBLE_PUNCTUATION)
Suggestions: .
, …
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-periods
Rule: https://community.languagetool.org/rule/show/DOUBLE_PUNCTUATION?lang=en-US
Category: PUNCTUATION
@classmethod | ||
def load_xml(cls, file: str) -> "TriangleScene": | ||
""" | ||
Load a triangle scene from a XML file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’. (EN_A_VS_AN)
Suggestions: an
URL: https://languagetool.org/insights/post/indefinite-articles/
Rule: https://community.languagetool.org/rule/show/EN_A_VS_AN?lang=en-US
Category: MISC
"list_sionna_scenes", | ||
"SionnaScene", | ||
"Shape", | ||
"Material", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Did you just mean “,” or “)”? (COMMA_PERIOD[1])
Suggestions: ,
, )
Rule: https://community.languagetool.org/rule/show/COMMA_PERIOD?lang=en-US&subId=1
Category: PUNCTUATION
|
||
from .. import _core | ||
from ..geometry.triangle_mesh import TriangleMesh | ||
from ..plotting import draw_markers, reuse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Two consecutive dots (DOUBLE_PUNCTUATION)
Suggestions: .
, …
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-periods
Rule: https://community.languagetool.org/rule/show/DOUBLE_PUNCTUATION?lang=en-US
Category: PUNCTUATION
}, | ||
} | ||
} | ||
} | ||
|
||
#[pymethods] | ||
impl TriangleMesh { | ||
pub(crate) fn append(&mut self, other: &mut Self) { | ||
let offset = self.vertices.len(); | ||
self.vertices.append(&mut other.vertices); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Vertices
, vertices
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
|
||
self.triangles.reserve(other.triangles.len()); | ||
|
||
for (v0, v1, v2) in &other.triangles { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Triangles
, triangles
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
src/scene/triangle_scene.rs
Outdated
.map(|(id, range)| { | ||
( | ||
id.clone(), | ||
PySlice::new(py, range.start as _, range.end as _, 1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Start
, start
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
src/scene/triangle_scene.rs
Outdated
.map(|(id, range)| { | ||
( | ||
id.clone(), | ||
PySlice::new(py, range.start as _, range.end as _, 1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: End
, end
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
}; | ||
mesh.append(&mut other_mesh); | ||
let end = mesh.triangles.len(); | ||
mesh_ids.insert(id, start..end); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
Two consecutive dots (DOUBLE_PUNCTUATION)
Suggestions: .
, …
URL: https://languagetool.org/insights/post/punctuation-guide/#what-are-periods
Rule: https://community.languagetool.org/rule/show/DOUBLE_PUNCTUATION?lang=en-US
Category: PUNCTUATION
.map(|(id, range)| { | ||
( | ||
id.clone(), | ||
PySlice::new_bound(py, range.start as _, range.end as _, 1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Start
, start
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
.map(|(id, range)| { | ||
( | ||
id.clone(), | ||
PySlice::new_bound(py, range.start as _, range.end as _, 1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: End
, end
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #76 +/- ##
==========================================
- Coverage 86.71% 84.23% -2.49%
==========================================
Files 18 20 +2
Lines 572 704 +132
==========================================
+ Hits 496 593 +97
- Misses 76 111 +35 ☔ View full report in Codecov by Sentry. |
No description provided.