Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Convert Japanese calendar data to zero-copy #1393

Closed
Manishearth opened this issue Dec 14, 2021 · 2 comments · Fixed by #1635
Closed

Convert Japanese calendar data to zero-copy #1393

Manishearth opened this issue Dec 14, 2021 · 2 comments · Fixed by #1635
Assignees
@robertbastian
Labels
blocked A dependency must be resolved before this is actionable C-datetime Component: datetime, calendars, time zones S-small Size: One afternoon (small bug fix or enhancement) T-core Type: Required functionality
Milestone
ICU4X 0.6

Comments

@Manishearth
Copy link
Member

Manishearth commented Dec 14, 2021 •
edited
Loading

Main issue: #856

Followup from #1116

Currently JapaneseErasV1 uses LiteMap, it should use ZeroMap.

Because it uses TinyStr, doing so will worsen the experience of using zerovec until zbraniecki/tinystr#56 is landed; after which we should be able to do this pretty easily.
@Manishearth Manishearth added the blocked A dependency must be resolved before this is actionable label Dec 14, 2021
@Manishearth Manishearth self-assigned this Dec 14, 2021
This was referenced Dec 14, 2021
Closed
Closed
Merged
@sffc sffc added this to the ICU4X 0.6 milestone Jan 6, 2022
@sffc sffc added C-datetime Component: datetime, calendars, time zones S-small Size: One afternoon (small bug fix or enhancement) T-core Type: Required functionality labels Jan 6, 2022
@robertbastian
Copy link
Member

Had a quick stab at this, a straight up replacement from LiteMap to ZeroMap is not possible, because we actually use the LiteMap as a binary-searchable slice to get the closest era to a given year. ZeroMap doesn't support this, for ZeroVec there's #1589

@Manishearth
Copy link
Member Author

#1605

@robertbastian robertbastian mentioned this issue Feb 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@robertbastian robertbastian

Labels
blocked A dependency must be resolved before this is actionable C-datetime Component: datetime, calendars, time zones S-small Size: One afternoon (small bug fix or enhancement) T-core Type: Required functionality
Projects
None yet
Milestone
ICU4X 0.6
Development

Successfully merging a pull request may close this issue.

ZeroFrom JapaneseEras robertbastian/icu4x
3 participants
@sffc @Manishearth @robertbastian