add ascii sanitization and update safe name calls#53
add ascii sanitization and update safe name calls#53daryaguettler wants to merge 1 commit intomainfrom
Conversation
|
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (58.3%) is below the target coverage (90.0%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #53 +/- ##
=======================================
+ Coverage 58.1% 58.3% +0.1%
=======================================
Files 31 32 +1
Lines 4707 4725 +18
Branches 425 426 +1
=======================================
+ Hits 2739 2755 +16
- Misses 1835 1836 +1
- Partials 133 134 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| def _ascii_safe(s: str) -> str: | ||
| """Return an ascii-only string suitable for idf names.""" | ||
| return s.encode("ascii", "ignore").decode("ascii") |
There was a problem hiding this comment.
Is it definitely ascii and not, say, utf-8 or some such that we want?
What does the ignore arg do?
| """ | ||
| glazing_mat = SimpleGlazingMaterial( | ||
| Name=self.Name, | ||
| Name=self.safe_name, |
There was a problem hiding this comment.
Why do we have this change? Shouldn't Name be safe by default now?
There was a problem hiding this comment.
yes, see below comment. These two were using Name not safe_name, so I just updated to safe_name to stay in convention with the Name of the other objects. We can poentially remove safe_name altogether
| ) | ||
| infiltration_name = ( | ||
| f"{target_zone_or_zone_list_name}_{self.safe_name}_INFILTRATION" | ||
| f"{target_zone_or_zone_list_name}_{self.Name}_INFILTRATION_Schedule" |
There was a problem hiding this comment.
In fact, can we get rid of safe_name entirely now?
Are there negative implications of the renaming at the instance level?
My main fear is with references breaking in the IDF etc
Let's maybe review together in person at some pt
There was a problem hiding this comment.
yeah, I originally removed safe_name from everywhere and just kept name in place there, since that class method should take care of the sanitization, but was worried it may break some logic so opted for the less invasive change for now. Let's discuss though!
3 participants
No description provided.