Skip to content

Commit

Permalink
chore: deprecate errorMargin
Browse files Browse the repository at this point in the history
  • Loading branch information
stevekaplan123 committed Sep 3, 2023
1 parent fcd6482 commit 113dea1
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 109 deletions.
11 changes: 2 additions & 9 deletions sefaria/client/wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,9 @@ def format_link_object_for_client(link, with_text, ref, pos=None):
com["sourceVersion"] = {"title": link.versions[linkPos]["title"], "language": link.versions[linkPos].get("language", None)}
com["displayedText"] = link.displayedText[linkPos] # we only want source displayedText

compDate = getattr(linkRef.index, "compDate", None)
compDate = getattr(linkRef.index, "compDate", None) # default comp date to in the future
if compDate:
try:
com["compDate"] = int(compDate)
except ValueError:
com["compDate"] = 3000 # default comp date to in the future
try:
com["errorMargin"] = int(getattr(linkRef.index, "errorMargin", 0))
except ValueError:
com["errorMargin"] = 0
com["compDate"] = compDate

# Pad out the sections list, so that comparison between comment numbers are apples-to-apples
lsections = linkRef.sections[:] + [0] * (linkRef.index_node.depth - len(linkRef.sections))
Expand Down
128 changes: 28 additions & 100 deletions sefaria/model/text.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ class Index(abst.AbstractMongoRecord, AbstractIndex):
"compDate",
"compPlace",
"pubPlace",
"errorMargin",
"era",
"dependence", # (str) Values: "Commentary" or "Targum" - to denote commentaries and other potential not standalone texts
"base_text_titles", # (list) the base book(s) this one is dependant on
Expand Down Expand Up @@ -307,30 +306,20 @@ def expand_metadata_on_contents(self, contents):
contents["base_text_titles"] = [{"en": btitle, "he": hebrew_term(btitle)} for btitle in self.base_text_titles]

contents["heCategories"] = list(map(hebrew_term, self.categories))
contents = self.time_period_and_place_contents(contents)
return contents


composition_time_period = self.composition_time_period()
if composition_time_period:
contents["compDateString"] = {
"en": composition_time_period.period_string("en"),
"he": composition_time_period.period_string("he"),
}


composition_place = self.composition_place()
if composition_place:
contents["compPlaceString"] = {
"en": composition_place.primary_name("en"),
"he": composition_place.primary_name("he"),
}

pub_place = self.publication_place()
if pub_place:
contents["pubPlaceString"] = {
"en": pub_place.primary_name("en"),
"he": pub_place.primary_name("he"),
}

def time_period_and_place_contents(self, contents):
""" Used to expand contents for date and time info """
for k, f in [("compDateString", self.composition_time_period), ("pubDateString", self.publication_time_period)]:
time_period = f()
if time_period:
contents[k] = {"en": time_period.period_string('en'), 'he': time_period.period_string('he')}

for k, f in [("compPlaceString", self.composition_place), ("pubPlaceString", self.publication_place)]:
place = f()
if place:
contents[k] = {"en": place.primary_name('en'), 'he': place.primary_name('he')}
return contents

def _saveable_attrs(self):
Expand Down Expand Up @@ -444,7 +433,7 @@ def publication_place(self):

# This is similar to logic on GardenStop
def composition_time_period(self):
return self._get_time_period("compDate", "errorMargin")
return self._get_time_period("compDate")

def publication_time_period(self):
return self._get_time_period("pubDate")
Expand All @@ -454,77 +443,27 @@ def best_time_period(self):
:return: TimePeriod: First tries to return `compDate`. Deals with ranges and negative values for compDate
If no compDate, looks at author info
"""
start, end, startIsApprox, endIsApprox = None, None, None, None

if getattr(self, "compDate", None):
errorMargin = int(getattr(self, "errorMargin", 0))
self.startIsApprox = self.endIsApprox = errorMargin > 0

try:
year = int(getattr(self, "compDate"))
start = year - errorMargin
end = year + errorMargin
except ValueError as e:
years = getattr(self, "compDate").split("-")
if years[0] == "" and len(years) == 3: #Fix for first value being negative
years[0] = -int(years[1])
years[1] = int(years[2])
try:
start = int(years[0]) - errorMargin
end = int(years[1]) + errorMargin
except UnicodeEncodeError as e:
pass

return self._get_time_period('compDate')
else:
author = self.author_objects()[0] if len(self.author_objects()) > 0 else None
tp = author and author.most_accurate_time_period()
if tp is not None:
tpvars = vars(tp)
start = tp.start if "start" in tpvars else None
end = tp.end if "end" in tpvars else None
startIsApprox = tp.startIsApprox if "startIsApprox" in tpvars else None
endIsApprox = tp.endIsApprox if "endIsApprox" in tpvars else None

if not start is None:
from sefaria.model.timeperiod import TimePeriod
if not startIsApprox is None:
return TimePeriod({
"start": start,
"end": end,
"startIsApprox": startIsApprox,
"endIsApprox": endIsApprox
})
else:
return TimePeriod({
"start": start,
"end": end
})
return tp

def _get_time_period(self, date_field, margin_field=None):
def _get_time_period(self, date_field):
"""
Assumes that value of `date_field` ('pubDate' or 'compDate') is a list of integers.
"""
from . import timeperiod
if not getattr(self, date_field, None):
return None

try:
error_margin = int(getattr(self, margin_field, 0)) if margin_field else 0
except ValueError:
error_margin = 0
startIsApprox = endIsApprox = error_margin > 0

try:
year = int(getattr(self, date_field))
start = year - error_margin
end = year + error_margin
except ValueError as e:
try:
years = getattr(self, date_field).split("-")
if years[0] == "" and len(years) == 3: #Fix for first value being negative
years[0] = -int(years[1])
years[1] = int(years[2])
start = int(years[0]) - error_margin
end = int(years[1]) + error_margin
except ValueError as e:
return None
years = getattr(self, date_field)
if len(years) > 1:
start, end = years
startIsApprox = endIsApprox = True
else:
start = end = years[0]
startIsApprox = endIsApprox = False
return timeperiod.TimePeriod({
"start": start,
"startIsApprox": startIsApprox,
Expand Down Expand Up @@ -688,16 +627,10 @@ def _normalize(self):
"lengths", # optional for old style
"transliteratedTitle",# optional for old style
"""
deprecated_attrs = ["titleVariants","sectionNames","heTitle","heTitleVariants","maps","length","lengths", "transliteratedTitle"]
deprecated_attrs = ["titleVariants","sectionNames","heTitle","heTitleVariants","maps","length","lengths", "transliteratedTitle", "errorMargin"]
for attr in deprecated_attrs:
if getattr(self, attr, None):
delattr(self, attr)
try:
error_margin_value = getattr(self, "errorMargin", 0)
int(error_margin_value)
except ValueError:
logger.warning("Index record '{}' has invalid 'errorMargin': {} field, removing".format(self.title, error_margin_value))
delattr(self, "errorMargin")

def _update_alt_structs_on_title_change(self):
old_title = self.pkeys_orig_values["title"]
Expand Down Expand Up @@ -758,11 +691,6 @@ def _validate(self):
if getattr(self, "collective_title", None) and not hebrew_term(getattr(self, "collective_title", None)):
raise InputError("You must add a hebrew translation Term for any new Collective Title: {}.".format(self.collective_title))

try:
int(getattr(self, "errorMargin", 0))
except (ValueError):
raise InputError("composition date error margin must be an integer")

#complex style records- all records should now conform to this
if self.nodes:
# Make sure that all primary titles match
Expand Down

0 comments on commit 113dea1

Please sign in to comment.