From e636398fac7749feb9beb2c19eb82e23c0240c70 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Thu, 7 Sep 2023 10:08:42 +0100
Subject: [PATCH 1/8] Allow for account for typer magic on optional types

---
 src/rred_reports/reports/interface.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rred_reports/reports/interface.py b/src/rred_reports/reports/interface.py
index a68ccfef..5cfa72a0 100644
--- a/src/rred_reports/reports/interface.py
+++ b/src/rred_reports/reports/interface.py
@@ -158,7 +158,7 @@ def send_school(
 
     dispatch_list = top_level_dir / dispatch_path
 
-    if not manual_id:
+    if not manual_id or (not manual_id.exists):
         manual_id = []
         report_directory = top_level_dir / "output" / "reports" / str(year) / "schools"
         for report_path in sorted(report_directory.glob("report_*.pdf")):

From cbed04779479630c27176f4f8ce2fb59aebccefa Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Thu, 7 Sep 2023 10:09:25 +0100
Subject: [PATCH 2/8] Warn if a school report has duplicate rows

---
 src/rred_reports/reports/schools.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/rred_reports/reports/schools.py b/src/rred_reports/reports/schools.py
index 9e77e30f..c3bae0b6 100644
--- a/src/rred_reports/reports/schools.py
+++ b/src/rred_reports/reports/schools.py
@@ -2,6 +2,7 @@
 from pathlib import Path
 
 import pandas as pd
+from loguru import logger
 
 from rred_reports.reports.filler import TemplateFiller
 
@@ -239,7 +240,12 @@ def populate_school_tables(school_df: pd.DataFrame, template_path: Path, report_
         columns, filter_function = column_and_filter
         filtered = filter_function(school_df, report_year)
         table_to_write = filtered[columns]
-        template_filler.populate_table(index + 1, table_to_write)
+        if index == 0 and (table_to_write.shape[0] != table_to_write.drop_duplicates().shape[0]):
+            logger.warning(
+                "Table 1 has duplicate values that will be removed, suggests an issue with the masterfile school or teacher data:\n{school_data}",
+                school_data=table_to_write.to_markdown(),
+            )
+        template_filler.populate_table(index + 1, table_to_write.drop_duplicates())
 
     return template_filler
 

From 9b7e92f48abf4b5b81cad90283f8bd423d97a05d Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Thu, 7 Sep 2023 10:23:50 +0100
Subject: [PATCH 3/8] Also ensure school summary is unique

---
 src/rred_reports/reports/schools.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rred_reports/reports/schools.py b/src/rred_reports/reports/schools.py
index c3bae0b6..c81fde9f 100644
--- a/src/rred_reports/reports/schools.py
+++ b/src/rred_reports/reports/schools.py
@@ -187,7 +187,7 @@ def get_outcome_from_summary(outcome_df: pd.DataFrame, outcome_type: str) -> int
             return 0
 
     filtered = filter_by_entry_and_exit(school_df, report_year)
-    filtered_summary_table = filtered[columns_used].copy()
+    filtered_summary_table = filtered[columns_used].drop_duplicates().copy()
     # let's try and reduce the pain with exit outcome labels
     filtered_summary_table["exit_outcome"] = filtered_summary_table["exit_outcome"].str.lower().str.strip()
 

From 36885a5d5dc25cbb212a3ddab515edd6966d8151 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Thu, 7 Sep 2023 10:36:59 +0100
Subject: [PATCH 4/8] Fix typer optional list

---
 src/rred_reports/reports/interface.py | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/rred_reports/reports/interface.py b/src/rred_reports/reports/interface.py
index 5cfa72a0..48f59b65 100644
--- a/src/rred_reports/reports/interface.py
+++ b/src/rred_reports/reports/interface.py
@@ -1,5 +1,5 @@
 from pathlib import Path
-from typing import Optional
+from typing import Annotated, Optional
 
 import typer
 from loguru import logger
@@ -133,7 +133,7 @@ def create(level: ReportType, year: int, config_file: Path = "src/rred_reports/r
 @app.command()
 def send_school(
     year: int,
-    manual_id: Optional[list[str]] = typer.Option(None),  # noqa: B008
+    manual_id: Annotated[Optional[list[str]], typer.Option([])] = (),
     attachment_name: str = "RRED_report.pdf",
     config_file: Path = "src/rred_reports/reports/report_config.toml",
     top_level_dir: Optional[Path] = None,
@@ -157,16 +157,15 @@ def send_school(
         top_level_dir = TOP_LEVEL_DIR
 
     dispatch_list = top_level_dir / dispatch_path
-
-    if not manual_id or (not manual_id.exists):
-        manual_id = []
+    school_ids = list(manual_id)
+    if not manual_id:
         report_directory = top_level_dir / "output" / "reports" / str(year) / "schools"
         for report_path in sorted(report_directory.glob("report_*.pdf")):
-            manual_id.append(report_path.stem.split("_")[-1])
+            school_ids.append(report_path.stem.split("_")[-1])
 
     email_details = []
     logger.info("Getting dispatch list details for each school report pdf found")
-    for school_id in tqdm(manual_id):
+    for school_id in tqdm(school_ids):
         email_info = get_mailing_info(school_id, dispatch_list, override_mailto)
         email_details.append({"school_id": school_id, "mail_info": email_info})
 
@@ -177,7 +176,7 @@ def send_school(
             school_mailer(email_detail["school_id"], year, email_detail["mail_info"], report_name=attachment_name)
             emailed_ids.add(email_detail["school_id"])
         except Exception as error:
-            all_schools = set(manual_id)
+            all_schools = set(school_ids)
             schools_to_send = sorted(all_schools.difference(emailed_ids))
             school_command = f"--manual-id {' --manual-id '.join(schools_to_send)}"
             logger.error(
@@ -198,8 +197,13 @@ def main():
 
 
 if __name__ == "__main__":
-    create(ReportType.SCHOOL, 2022, TOP_LEVEL_DIR / "src/rred_reports/reports/report_config.toml")
+    # create(ReportType.SCHOOL, 2022, TOP_LEVEL_DIR / "src/rred_reports/reports/report_config.toml")
     ## test sending reports to specific UCL user
-    # send_school(2021, config_file=TOP_LEVEL_DIR / "src/rred_reports/reports/report_config.toml", top_level_dir=TOP_LEVEL_DIR, override_mailto="username@ucl.ac.uk")
+    send_school(
+        2022,
+        config_file=TOP_LEVEL_DIR / "src/rred_reports/reports/report_config.toml",
+        top_level_dir=TOP_LEVEL_DIR,
+        override_mailto="sejjpia@ucl.ac.uk",
+    )
     ## test sending reports to RRED email for UAT
     # send_school(2021, config_file=TOP_LEVEL_DIR / "src/rred_reports/reports/report_config.toml", top_level_dir=TOP_LEVEL_DIR, override_mailto="ilc.comms@ucl.ac.uk")

From 8556e58ad25fc93df23f0c038dc953a7365298e1 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Thu, 7 Sep 2023 18:06:55 +0100
Subject: [PATCH 5/8] Test logging of warning for template filling

---
 pyproject.toml               |  2 ++
 tests/conftest.py            |  2 ++
 tests/test_report_writing.py | 16 ++++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/pyproject.toml b/pyproject.toml
index ec97922c..98729596 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -47,12 +47,14 @@ test = [
   "pytest-cov >=4",
   "pytest-mock >=3.10.0",
   "email-validator==1.3.0",
+  "loguru_caplog >= 0.2.0",
 ]
 dev = [
   "pytest >=7",
   "pytest-cov >=4",
   "pytest-mock >=3.10.0",
   "pre-commit >= 3",
+  "loguru_caplog >= 0.2.0",
 ]
 
 [project.urls]
diff --git a/tests/conftest.py b/tests/conftest.py
index 126c385a..62caebd7 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,3 +1,5 @@
+from loguru_caplog import loguru_caplog  # noqa: F401
+
 pytest_plugins = [
     "tests.fixtures.test_template_files",
     "tests.fixtures.test_redcap_files",
diff --git a/tests/test_report_writing.py b/tests/test_report_writing.py
index 1c5ff0a1..94a37f0d 100644
--- a/tests/test_report_writing.py
+++ b/tests/test_report_writing.py
@@ -57,6 +57,22 @@ def test_school_tables_filled(example_school_data: pd.DataFrame, templates_dir:
     assert output_doc.exists()
 
 
+def test_duplicate_student_warning(example_school_data: pd.DataFrame, templates_dir: Path, temp_out_dir: Path, loguru_caplog):
+    """
+    Given a masterfile dataframe with one duplicated row
+    When the school template is populated
+    Then the resulting table will have one less row than the input data, and there will be a loguru message for the duplication
+    """
+    output_doc = temp_out_dir / "school.docx"
+
+    duplicate_school_data = example_school_data.copy()
+    duplicate_school_data.iloc[3] = duplicate_school_data.iloc[5]
+
+    populated_template = populate_school_data(duplicate_school_data, templates_dir / "2021/2021-22_template.docx", 2021, output_doc)
+    assert (duplicate_school_data.shape[0] - 1) == len(populated_template.tables[1].rows)
+    assert "Duplicate students found" in loguru_caplog.text
+
+
 def test_school_name_replaced_in_paragraphs(example_school_data, templates_dir: Path, temp_out_dir: Path):
     """
     Given a school template with the first non-blank paragraph having a "School A" placeholder

From a2ca2f36f529a44845007d6bf7c07552c99a37d7 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Thu, 7 Sep 2023 18:12:41 +0100
Subject: [PATCH 6/8] Log warning when school has duplicate information

---
 src/rred_reports/masterfile.py                |   4 +++
 ...masterfile_with_school_in_two_regions.xlsx | Bin 0 -> 12781 bytes
 tests/test_masterfile.py                      |  24 ++++++++++++++++++
 3 files changed, 28 insertions(+)
 create mode 100644 tests/data/masterfile_with_school_in_two_regions.xlsx

diff --git a/src/rred_reports/masterfile.py b/src/rred_reports/masterfile.py
index 50ef24ce..0bb0ece2 100644
--- a/src/rred_reports/masterfile.py
+++ b/src/rred_reports/masterfile.py
@@ -129,6 +129,10 @@ def clmnlist(i: int, data: pd.DataFrame = full_data) -> list:
 
     all_schools_df = School.new(clmnlist(6), clmnlist(3), clmnlist(4), clmnlist(5))  # pylint: disable=E1121
     all_schools_df = all_schools_df.drop_duplicates()  # pylint: disable=E1101
+    is_duplicated = all_schools_df.duplicated(["rrcp_school"])
+    if any(is_duplicated):
+        duplicated_schools = all_schools_df[all_schools_df["rrcp_school"].isin(all_schools_df.loc[is_duplicated, "rrcp_school"])]
+        logger.warning("The following School IDs had duplicate information:\n{duplicated_df}", duplicated_df=duplicated_schools.to_markdown())
 
     teach_df = Teacher.new(clmnlist(1), clmnlist(2), clmnlist(6))  # pylint: disable=E1121
     teach_df.drop_duplicates(subset=["rred_user_id", "school_id"], inplace=True)  # pylint: disable=E1101
diff --git a/tests/data/masterfile_with_school_in_two_regions.xlsx b/tests/data/masterfile_with_school_in_two_regions.xlsx
new file mode 100644
index 0000000000000000000000000000000000000000..7c2343aa4d07c62f5d958a9e31029ef69469a044
GIT binary patch
literal 12781
zcmeHtWmH}14(P$%iWaB1ySr;~FBB>6oP%re;%<XmX>oTcR;;)c_u}sU4xPDoW_stn
z_5R<xS?lbz*-3ViFZq%$iLyKtG!_6B01p5F$N-|;{v&n}001-$0DuXAhkPS$Z|7uc
z=VYMn?qCYiXL7T(A<cz`q{#+Ag5m#n{TFMXM7`gxlLft1YgbsJO*1qnq^Jm<u!-;u
z#Wq}uTbwpU-R4W0dv9tp6^%;x4oVf9kvCLl0(hl!R61fJ>o!ccI!Y9VNQIH5z!Sj-
znJh{F{1VN`>X>lOY&?{sBiM;rIE98eV<xLtPp>F(IBO8uw>nN0cf^W62#Zs;()}oc
zy!pN-*x#uGDNpN+XUyXkpvs6Zx1`n5qQb_mNl~5#BGO<=xQe93s#I>Qxh0X^9T>;T
z)~~d9$tO-CVp#(5$(D@-nLQW*y^<B{l+^q^EHn9H(?A0oV%MCTm*Ji?UQ@<;S)H@N
zJ-`20OoT?Z(o~geqt!+<(pf6#Y@egApfr?mK-Y9%@e1M54bRl;!k!`j4A*aDn>3R(
z=ES`<5FQkaZn+3pYC6C;Cj^y3ZX=wWYjWcr<K88pMoseYM5oUIl3k5(W6St1kfHsM
z_r-x0-!CHwZ;Izp7c+82Nmi0hPzfHve)jYP1yKH*o7QTuke`EXPXYWE3GAi@KvNqK
zGt=|$|J?Jx*eCxYdP&@Cr7jla;3Jv)kiM&_#Taxkc~?=HW-@i3_p%FUwUK!g1dHv|
zgy`z{0Z`Jutv>ey^NRwJyM1JrYaAuvFR=K@>s(8NlW!fJ;pnJ<Nm7m_tKAqbQx{X0
zsnT-pG%l^tbj1yya^(A$$)zSvBr4HIn6&T_FbfESaRgKSbicmVSu(yZgP0OiJ17mV
zsOQe!jUP$#ok{w*gChKaS7CoV6}t~)Y(87&)n`L?af7d+X2EM&VVnu%d*yCmV)Nxp
zBCQ?s!HZEby-)QO$BS#W5$Rsa><eGr8jh2}4EJtsgs$Sg<9`2el21$Ei1nXN;wO}l
zd<ND?6WB@c0q_uRHq5{KiK{)(+SuOS`Z=2Y%V!|KAq`CBzxygteJ$S&-qMjCLs(qX
zT(B`_otR(kYwV%H_t((QQ<1UzoG%m5HR)?l%QHjS2fH5)^}1YfqOL(<UUblZ3VQ+V
zfoXHZ4`V-cHUtH4d{9d4FAanFVt;q9_y7s@HxOo{P+T8FCKk_+YZ}UzA$cEPNt`#1
z2Xj~rvpBO^fkuf2z&0~jv+@2mP;I}|zox#P^fy|w&8J53`AgU=e|M6xvzG|60x0f)
zvF%AR`s)tKOZd~4d$ZT>!`2;nx5%4>#qz{OoPu7#PNX4y(j>mLaiGoI)MkyNRVT5#
z0owNYQ-;CwBeRYE-l=8qO8=+HtdeI2wLk#?DhL1o8W;j>GJiFe5)BpmJRWo(;hjgQ
zkjq4TOj2l09Z`UOfs00grRSA8uVI+Rmvq+GMOIIDk+L%5Lpuq3ZxiRf6ASsWpA9+O
zIIAVj(PJ~NU#R!uek>c~+k9D^Re1Zr#wwt1P5GH!_4GHz3+}mH1M~JrVB?Q9)1I4f
zSb#JC6cQ}~Ne&mIh2$L4(kQ+nkcsY<5pAZ*JiJ_0UADR=WtGB&B(_m)o8)m1FP^}N
zjvRTW3wG%yj>Kr3O!hKPi6vYOogiG<tF;2_hy*PZkrM6xjW=jT)*8D8f-`zC8VGjw
z6?wMM_j=d8RLIi~n1mU68kOqDNn59o*<(rPp-<l>@O<GresI8Uk<H4CYJCJYm{Tq&
zRROQG4<V4#gd>ARbz3#P;FqY!(gnHYv-E$I<fFaWvC6k>i<L7inyihhVH-}#pH(O6
ziYwm<K^fyT>XqZ^`500eW#-AdjL4YByCc^KO@?-CxZQ*~sFFk)k)5DEDKXAlPPy%{
zH8WyW)|-h-h*ie8Z9s&Hg@|+x)7B}YnpHCQ<`q&Y=BB#z6(diyt}Mt&xLrzY^c#B3
zs#u+je=p;}2a0W(Pa8FKj{)kkMfcL^?-~0K&Z0X2Wt}7&ZuCL0BU)pe`5RF!V*Vnq
zvt>Tz>abEu&U9Y)=Rz+2Ul15(Ql!bazBLjN7;%sg<c01;V$L8qM6?I6cV4I|^6BgY
z%d-ocP(p=xo5<2uq0@YNtDquXL09IVZbFQB?#c)?^$-2VoIRC8b*<8*HO;*iJ<quW
z_^Gbz57CHqsKP(nxi1+VxejC^8HV?^lBxR}b51Qgu2HxcD@ITtVB*;f9_vkT?)Tw)
zYO;h<Crwq0N;}=YMXb5iDfWC={`?r0V_DpYR#Apvut-)`^>)|74JB~c>bK8L5HWdE
zsJoVG(^EZaoe2x%G2>x1>WOM{1CRJHXoolqgP(3awyO^g+P!7l3`v4xJwu}$L|I6V
zptG^)6kiKYA#gD&^`zTvIMd&~>>gHsZ^Mq4hB?o&9{cY55;UYqG{ZYojIazx!k(PG
z$(%N+$EUIQyWh@2M$(OS(8-eh^O=tw^+p8(EocfgUIUijRpqP){eY*LLr_E}P*NaA
z)}MFCHQ9)8<cQAL5YcSR@~tBG_~e`RVpg<XIWpJa&Pu_h`EJOoxl;ZpWZ()n7HD#H
z+5U3L?}}1zg2$a_!r}42%pLUcz`4cCLt&T`)vl9<XIV&_!CyG}grp05n*^%qbpyU<
zYOW|%b^AZY28Rz>uTa5`?t}yYVEyjsAPXa)sfn5s(9+Hv^c)%@G?$g;S<rp5YMwd<
zCO(QzB$d*uy_r-@yJUfYq+01`+lkfYaj786etH<T1uzZ=T@(y>FF(|caPxc6VS57h
z^2g3hIPj>Doy?|Hx6U?3dPW{BJJWGJ7!$@=l(<&n@_O!E*|!A5^$&i?u6kcEzSh?i
zp<Vc*o{iDSS%*a|Y@69M;IG{b`6Zc-2rVSO21}KifxY%}+m%mNyL8hVW!W`u$EYfW
zw=v}=C@B5SVCy-e*=K`q<D~X1X&PSE-FAGeeo_1o5z}~6!7i#yGqxf;aKo~wd{-;?
zDV+sD@1t*zU&BzmMVu<Ox5`RoE9wHdF|oJqq9_J0ufR!M&)-)L%@QSkkq;8ON`+_j
zaSvy44?n(O>`b8Ga32@t^3F%3bSE_=m+h1%lM<&3(?Zl4!W|&9%}3xIfT^h`Xyf9x
zojPk`=%_0|(ZAkyIEWdocE-J>5Tu4!YdKV~6G1oH$evOEu!6+MzLSU4&R!HaY#ocK
zqYfKFRIn=oB6MkUS?qDdHc_P^<O!;5O9SOKz-ZA!YF6P6OH73e-}+qV;4+nX?o%$n
zORwe(HZk3hf6>>=G*@4Z;b<bTXL&1^USORm33}<D$W<IQu+VY(WIE_NpUYo&A;HCv
zE%<=QvRK9_=R%+DJ%q@!SUhfFK<KdB$;FcD5+JNPCb$=GZYrO0ewTIZ^Mtu{L~<ef
zL@q=eC4IMU!DaCNOYdOSazn(*GPTQFE$k%|7qhjgHMdzl_Z{K1-bzc7Z<rYbdTDcO
z)`UO?#7*ilxyq42F%QE#8`Fa1GD8x>7_{$b8T9va@3yX+;BJVUp*O>#ZM9G;8HECn
zH=u%pVLQjWG$EN+K&Q5sDMd@15?8m$4mL2^v%qHJoUy53#@WyF+2p3cQ2E@-{qB&W
ztfP<SF&Vlbf@#{kAgeeDtSWzfQZ@Cy(70-x5UGPNE;i;G`m#OFlX0cP-e2jBBtyef
zBiXdMI4!k|8u5u0%gg4B7bICNh+AhO1+324?%zh{H*hll^8v&}?L8_5*nf<{WdoEy
z{Kv`N#`LG-lxPmx&GVrz;%*D0FVH7o_1Egl+7d~N_${nK_QM?J66ogIyz+v_N*rLk
zJtC5;V&oPjv23mACv*StE&b(qf0`oV`4@$*S~*9D)py<MIr3$7w2wzjS3mGLlZxUH
zOSQwUN;>STl9}Va9>O#)vYKIte4^^%x89>9dtLc1jtjTR3{jKLmZFx0^Ue5V@K}{&
zNIq%K(a48<y;TS!+HeDi*vW<ZS}hA5wLxw@hvuV7mbjkN2Te<HZpcqAq7q1xFzlGQ
z!-VPQ)6zZ^%n{%U2s0pgyJ!u@Q^8dfClH64Ld1ty6&+JpAiR}*L?Y~i5!yn3NVP;Q
zTXLOfkX<4SZ!{H0NegoaPdE#{dgFLOJeClNAg!>6>CG)8`1~uC&hWX0uOkS(ad7AK
z?T!+it9$@jJ-z&IBinw66@jREqhbUmIXGV=R~W+Nxb_=z{q2!mzlO;%T?RA?&m@mD
zC;`!{sX5$uuGpXWv-6`dQXI&lQjy3&!!eY~CP^<7yPzZ2o4rw3X}3fC-n1W4H`fOr
z2C!rjlD2j+%vz|4JrQw$GU3#c;5BvnVpr!g?rNKYk0R@EtT^Q56zg?WR-yUR-F7L?
znQN@V4x(_c-V4!)zbTeBk~vyQK9-h7$z~z5BRYPX(rI;`%Aa20a-4NG-3pHR<j8YO
zNEP*jW&0)bi<HRim63`T(TVSs6ChD2Q>vz{$7d>><HsJdtxaJT%ni{}h^Adi|6TLR
z?5AjLpXgiYE1`*)`Ox>~r$j*_W%_FO9V%GyPByws3|dMua4Dv)bg;`)P0m`522|3_
z%J7uM7WDgzj;uX}C&DEgrGd?63m443qje!NCzf989<Ne(NlL1@Q5J-c!*#pV?l(Oy
ztSZ}v#qAHvf4p_-H@3$ppMFcIUpl?#wHop`HPat(3^yYB<Rj`1J645pY>~qHJ@)>f
zKWVMsBzt)oR>X6_8)Nw1)p5o1rhvq4EMmCzW}ofylm~vM+QOM=>m<<g{+oFhwJ3{#
zNR(GgjUHRalxT5)m>T^D=m`>NQB=8@iL8Kqf`80A`EA+cJHT(mz`=_AcXxWuJ3$tv
zrcR(gPAxwlSpR)y35@Ot>tw-_yn=cZck_&&4nh`J<9^kku?1y(I|n<T9CL;B;8{&*
z`ek#|VPuml?M~n`ff;Hn!(<a=(8fDli;82<ImUL;N}k;XECfb@7^4ZMw%?!>73D&E
zW3;i6DuYxg9`L$+x@Rk+5I)BU{g{;vb!X|n2`3mWDYCLXHGr#OR;)c@*znd0;1g8U
zdEAfkCK0g9Yaf<59SM}>jWHC;mErA^JUD%CEV}1<nQ85f3jNXc+u+$HIJf@K%=_ys
zlk^}W0AS4r0O0+H2L6;iJ$^oFjs-pVi2j1;)v`EhV1;NPc1VH#XWXevC);#{$`9+4
zT+|xgS5Y7Ug?gYSBpc6TyRVr*rT~fL>b|HY(_rAWcQlVOTk0t=K89pjtCf1qLVPd*
zrz$WoT_k>Mrxz_gK>bYy6doKxfuf9^0;h}q5D;Dc$`ppOtv~<8<cdM9qjSOgkF_#b
zb}R0&^fb(};&{c<cx;0_0R<-f5CxW`h=^7!z8Pn7t{ff29SB8r%|*TWpU^9Rz;GAh
z%Mo^bEl*RWYhtQ6%p+0#L5S@Gd5_AM=JD`V*5PA*QtA%*tSzvOQP|~Oauxed9rz^X
zvrYFM<oO;~{kSdRel-<a&oIEXGnf5aoqVLL!0SGeku2txDVwRY;Kz!3z2NGl37TV%
z69J#lH6y~nuH<$TK3U$(iqZj<7)p~L(OdR8N((#rnb}!{sJC=3$ojqkkSP#OC!z05
zE@0TI-{;EoLE`RrmlZ7NZqgr~j7rQPT8pKwdRn;nc6wbL&EbeUIgKREIGVQ?nco)!
zr`5Z}aN6ChfZrgFR1se;V*jwhz_i3&-dJTner@Mq9f(qe$)%_yKZ-0(aJC0Goz@Q-
zc|3!$@Kz;V!J{NAWE!rVX5Nxq(tKe6A7WY+>39}VYE@qpy&jRkiP)lesDCMt>2uQ3
zCVRv2!;@{4!>3(6o6)#{Pr@z6%f%F(Oq@Vu-J2Dk6LNt7UE_dZj3N8G&1vsPxMvgl
z>1hdX^g)=?==byqz9Ke@85ALGWph2+@G(QDj$SQ|gj$=S`7DxgzFGQ~1!;#|>&@KT
znTfYj?DOm|(iflHU9Z;He)JCA%wDxWx}QGGj_q&vKP+=EAD(4pGATu6-t+I-*=0SR
zEcYxv94yw8j6^B=SVs+y*vZ@Ub-ZjG@qKdlnM)^n5)kHZD>Z=z25*4mGf7>Ic=@h6
zf#<==N!BNvTAP)1Xs)C4M&0tB9#IQ(K%|YgI(DH8)-n)4dxk51AVD9XU<*-ZV1zR!
zA-i5gHTXqLLb|Jns>>3T?9L5<Vsnc;rZ6RtSY}|9TAnK*X9Nt=29SMui%4NgBhtv7
zkPEn2QjQX%z1DjlX_*qNik*E-FrYzGFj6i;i&OvuRIpwSV#Bqf*QR8oWD#MnN$3C^
zjH)6+aYzjE1}0hXhKyn{ZlTZRZtja3)TFnL<2gapqzi6GC!N8fj5~hK7UvtrtC8t_
z89=<l^KO)a$h^b}!N4Y++af9I40dtcaVc9L3~??m{W5%%88o0B=##4U^%Y)sdMON_
zsp89U!x!LxLZ_vul^O3u1xS9Q)xvn~55$%?7&T`tjOZCBn(P4$6P)+?pTP=Nt+XT(
zlc69D6FUv~SHs~{t?Y|NGw^s8NMA348Ko`kY9`r&*kB3Tn76#gGA^%z@uf);pllsk
zrSMkAHT`zuJ`4rlLM%8>8I#b29dmBREQ)~LJr^fm6r#1%KSO-;m7JT{eKU3@bFO}=
ze<Sut;IviR=eRGLc-C`+I{JP;)<EF2SUPy6YRpu=*1kMbby^?P=_=iaExh0@e_4U+
z(6V=Jj8br0-_?70(X5#M(Fw5VjpU+Z5lG2T=>o}5S<@Mp@xsHXpBQ1N!X!G-F6NsE
zKZEJ%cfr~VZ1<oA9JB>aGVX4!WCpIQPOkKjm<8DR*T`@?F$60e$9i`#$+Fc650^N;
zGVyONJ4)|XaC{u-g_{yd76+o;MyeOZW<jD~RWC#QM6?@Df`aG`w6BJv3npjfblhpL
zrSa*SaM1P9J{0VeOssf_LbJypH`t+v6W!k~8tL^Rja)l+*h$N0jxf-!`%~fyFx8+%
zAsu@-aq2<#uS3QNFui|4jJpEiu6L~1cG<^G+<4MQAKh{}*><5w745YZdn9-oAnkMB
zw?r=NzF{7HzZPpC$OfGp$#f@BZKju!aeV3ukwr*wgT#{=0>M?R`rx9sLdjfmS$@2C
z$K3*r!o~!kUx%#t^EMcORntXVSa1%t=)uP%;5HMufIjc&x6E?Y*|pw@x@A6yqTG1{
zHB?|=cvJRC{lQ3gg|JzJWT6VpZV|5jIPcB%7~{P*;?qnNnnwAAzb6ipR|d{Pwek+o
zrBs8QwVe2<aGOuWiIEfs&CN;IAq%5Lq!8xH#xWpvLX6u;S37a#w5#Lfkvn>jXdJb#
zw);GyyOxwdw9G{Y<hGfjfQ&3~MjNf{e}dui!5cbs=Vhq1D3?D)!26XRtPV;%m?Lz0
zZDS9qHQks<(tc7>H(p5x1gQ@GZp5UN%-9AT6LJ|gczB_VR}Ij4D@4XH@(Lp|dPr3m
z6fdEnpM?GiHghveT)O;au`P#OS<F<4N~_3C_~z;GW$l_&jtLpo+qNX4ySziIx~CVe
z4X1R_os>n%0XjU=rmK*YNIAz<kWsj826&Oj53Z_k+a6S4^gmy&Nv#JRwGP5Bi^e#H
z?wTWmacr^VaKu*Qzd3$9)FwkOd!&J@c5s#niVIS{rjSqnOsxqYSWG|A!Jy}@^89t7
zeth%YNNmW}-6$Xnl9sPJjShSe8cw5EQ(yAOD<w%y_KQA5KL=-7-jtwXCAtKLz)rYg
zLUO%U;72nZGIr`qNYmBbWW;AqJJ@>RwMNoETRwlnR!+SQb&7`(x-+eVR~mj<Di5UD
zmlP&-0l8_fGR2Gi%wzy43ZrazcK=E8tDy*X)k|C&B(1IA85ri3YTz@^8`dF1XUc3y
z`gG4(#a9RxVI}=tU@?emH~pF8^cRUi^6FLSh$QKt2nXk3DcnIYtNM(TbV$jco@L|N
z!UID^mc@gio0R3$amwfm{v^_*31)Q9m{vunp)BcHXX(`{TP5gvA-m~7mSwu#^q*46
z4$po9Jp*RaK5J6(cTH~DO!Cr0cRDX81n4x_Jwy5(yTpO)^2T`SO@ge|nU(eEYgC?f
zsRSI>%%~P4)pJ#RRw3x83aNR`i=SmH<1Zxq*6IAa(0_;A8~&^e7A<Qdy_C;9NTlHq
z%oNg0hT89o#{0&37%b}|@1`LRn21=v7>!pPsP3m;nn2IX>-uj_r=B^T`U&|9O}^)^
z|FIZg={3O8!)t9li^TU#lC&jlBYocDHp9H0Pj?l63Ms*NckJIB@c-n%`QK92VEPE$
zbLT$&EZzJJ;~dfRR?poGgPv`n;)&^l7rSCg!EcdaJt50gUQ`dWg;hblI}$3)bG;4B
zKbrX(Eykn=qr+N<qo>o{gE)yh^Ui|dejDgJYPW@GE2l;nJ*a=ukhzyTr&Xw7T68m>
z?qgK9n4kGdu0f#MigtjPIXQcSKPN~AE{vp4Jliepf=I~yP>gUA`%Psw$#gc4(?ctc
zTzwhDh1{!ipF(2q0D&>cs!-s$ZwlK+Gc(OQ)ozG9w|+6CQ0?z<{lOj{VI!}3ds4h#
zw4(^-+WelF{Myp%z(sFU1m`VZNuN9KKTFR}7N)kQ%s+pB=KcFRL*WG6SnasC!pI=k
zd(QP}%B7VNoA^ZvlZ*ty+PZxeO?I}p76MF2O5O`qn*4%fF?+#;aS<q#*7Iltikcmg
zG3uf3<fWSwX*P1YE-2yBkFAA;SD8K@XHF;MttlSTDM+1hNd|4or-JDb-%}_il6<Xs
zXgIu-WD+7UOcCsEkQnae=d*l(7`FZJz2Bj3By4h$NMw2E>2P}BgJT=MnDO{6Q$_U3
zbdqP`^b!&UET!Pk7pRuvk;h%X^*2`b^kyR?-$zN5^Y#4PqO{5yD~1w5v#eEANtsi*
z-c=>`A!u9mkce#V)F_jcpLor4{EFtq3m=Oc%7-o7dS;w4O)7~^SRHY@>?gKH1w$|G
zG0^Ewht)VKe1NWJfdSMfz21wx2i^9%@j?g-4*$fDb>I2+676D)P?T?{a+<Y{GzM{&
zMh*0<8;OddgJ!bTeJg@VkegN{k;+ABh4i{YGRz9i_hz!O4Bm9j$RE5_u0OX{mXHND
z1TzKITDdmPVViVrM`11@2Z*-D(T=FZpcL<`swrr{;@;GaIAjw7yoXe!kX`HVK~@@t
zuWAUV;a%yTr<{O)VITa0%CJG57DZ0ux?ITb@gsRy9Sd82qP}6MHi7D;n{ju)Lft)m
z_7pcMbEcr@<MGZ}NV}l#`SwkhK~-ZdoxB|!rJCB|RJ6g<!vpS;LHpzJ?g{tKB=t#W
zcl-U#U~>D@3ExG<+5$FnoA>GIPCV|i_r;b{G|}8VAyLXPS!m^P+3LF^s&FGe)VmW1
z;f!t;vF<n#jBS@shl72PZ{Lvab~;xabj}lMe}!ogXt$niA~|WKh)OM&s)Sj17vX~E
z%C}PV+u-rBL#D!F{8?jYbLeQS^&yj73E*mrJzHx{bvClX^ao1Vi_nie9ph^4mEsXZ
z#m(X$HfIFc))Xp2&kxbd13pYpiUSQPRheqUjXw-_nvw?2oenM8mUn%!o)YvjAvXf^
z!Q%7b^XAt<3nrC)RbB~JPNKGOlMObYZ-+Eh88d~1M;Ptd?K_4*`5$~C7x<#>h{)@x
zcf|VLTygTi?eWHNJoZ>Ym+z6v0-!mCxV~;5DpWGbSl0kqT(9U^ehEQwLggyfg?fa#
z&Ihws4KN#4za>zAxHTOns%(i!Z6(2V@~;RtofD}0UON=Z&d+!DIdQHTPt7UfE1qY)
z5J-E}h<$LYaZKx#VItAKTnT;Ou*OBYA%tmMk@Yk!Fd3fS^y(zRw!+-r(ccY+CKO{i
z{{~-TQBI#{GhN*yBMSBP3Mb?y$wv4D?F`H|OI&li>|P|ij(M^N&Y_pS1HF%Zb+4hp
zlHfrb^h+<Rm!y~VmqX|(*8T!U!SIO4xQqTDupjj#GKzI=4inP8r*WY{VM%GWV#aSo
z_Kbv9QQx}`5W7NvU}2))oZKbwrO}vfN%&PGT#+zXdst*Hd~t{?0ZqGWfZk4BKiw6X
zO}rH<HyyT4J=6Vqad{zXmD=LvH$ZiCx}B-iQn|@amT))-)6*>G>J8m876HfdX<ns?
zy3q<^_|^4qoGA<o3FM6Hj#R?bY;)10$hzXDi(k^i6fV@=*r~GL<%c-aV`q()-YK5B
zMMq93nm3j-{q`9NCho?KRxeUKS?=SuuHey9>Exczwz{iMGAzyP1fG$C8G(v3o{9mg
z7}i_!z_+uh+G9@IW6Hb9Il5Agn}{s7*w}8Hmn)L3n<1ieqQPc_30v>53u{taBFYel
zQ>8rO**~X3C>t}glSVtK%{Do@TRB@6wMynQl1DgIN3x8w_~WiJ@DC;3>3nTU4Vh$9
zYBX*n0J85!qG4D|zM^jMqoPt9?dG?;skMTyrz7XEzQNqa>zZ+cZ6u$3gPW_?;PO&7
z)9%{UDCs3`Rft3apI*G;04!^4if2MMqXDCDA_~VCVXiH`)_Zgz-Syu@=t<HX(Vh7F
z+}JwEErS>FR5BGw{8rV<93G2$mWfIzW(hl6Ng|_Lq4>7AG4i#wnd#x8Lf?l60C$I2
z{n9G1V>fSH*9-?NW?I6=c@v_@`NEZ5a#Kt)J2^wkzKto-li(@M{??JR$0U$a1a;Y}
zw8%{|H2{LO{9*pFhi-S$DSjt}e*>lXRD80EuE6d=kr7yC&D4ncV#-~vWp&a!9IAtb
zG3y)KYn>LwuCVHAmH8TW26$wpNv>;KXDJ@RlQL@IzKn0W&LRwZav9gR0}CxvW^k^4
z0v-;&JcX*;%3}7(<TSVA1bqoyhT#kGI+5_pExgo>Pd>CZaEo%^858i0+R@bmRC6%u
zii2G6GzT9l^md);&{hm%RrF9N_HH5cj7BA0EX#Xmm=4!B8Ha)}!ow8|zTwuc-mbOj
z&3ZW=VUU7LlLOg0wk}pdX0?Je(5;Gjqx;i*L2pan9HDqDR*Vq}9Ji9Apw3O%DerM3
z)&lF_z*|w8`sC?+f=iU+|5Q)#ko`bP|IToe*4v~FaAnn8_j^|HYxw=1cCnHR&Np6z
zi`wYmVF45ScgjF}2N1K7gTo(FZ~q$+0H-DC@vrUYpT|m|E=iCV33wNkk^ORw<+;x*
z3?brN%oRVr%c(Q@P!hX86J4h=v*R)sNy7YAFrNbCX|-F@B_R}CtIGsTG;*jsUTx4s
z3$fSAQkO!`dCv!{SIi#QRC~0XsvpKkbt#jo4Xs-o)~Ij!x~vn{g6x1agY6@%_PEnz
zB|~dh^L8cya!L8wz0`UrE0(riZF~+skrvTYxh01Fh;(!ai2lp8m)-mvzhUaQU8{Vo
zasMn%IvU~UfwoM-X{ixYDW0qC%=oS9p~%cav!m><k%UI_pAaH_7*jk0eZ2XPLrZqt
zrgGL(*iPF`{buOyU;+id>pI5>K;wLQ%S9!wSKr<L7_k#dCqb1b$$}H&P?pjAn5T~l
ziNF-p+5!J6QocK23x7wtF`)a2^zCpcTyU&w_w1F|dMc>R$b@$%@(Kr%kX80w7+0kw
zS$YA7<g&M0ZpbD2(fbqAu4KvTa#AC*WVXdG>MU1P3+mzhxU){eho(XWHJ?cdE8Qxu
zo5zIHdrw@yZw|eBiW!tonTqgvfV+I2SNPAmYKWVM=CfUkfbAONuXg>;9%%YM!v^c@
zk1H_l55tDJ1sgWe?C>osnWLZGwq*w_hh+GodYIzWTpTH@^TQGKd}zEOkqc&z7FxfS
z-hvm0^J$cjg>@1`mMMcMl%{31PF2W-zQ<6U$T*G8n*jM~BSY@Zn_0+oz}`u6J*uE3
zvMfa^=@-UXq*%;!IJXX|U>)_8l#&vAik?yXhWKMD?!It_<_(lM^*D+nzt3qOY(mso
z$QGw(phO^g@d-Gv+=K>iMCG^Bj&}mcK>0Qt&P>BK)>kQ|h6;+i(b?2S^L5oOLNyq7
zBR3Pfy$>u_MX!&rN$GJBp)HFl^^Ut(GdnMDu>HR7(SATUrs=;07Ie%T(#!d1Cc-KZ
zL*7DEX`L^%8_npJi*2M!VC1vBpNU$Fj?z$M%56&W(>w{MkKC=h)9Pps^45<&YFbXL
zJS0<PY2a7%qYL#39b5GN;rMuPcN>)`srhZ9L?*o0*@yO20qbn}!HxRJ^XY52+03?Y
zsKEro3uXyN#x^}djH~xRZ5`Q$$~J=e5L*1YS!KOK&$yb!S;Wiu;`I#Xe?BjbtVq{%
zf(@Gi8vvjNuOWC;#{t~BcQOU4nL0T=_wavxW@R$AwA)u7l3P&56J)PE*#3x$wcw<Z
zPw%*m9RGPyBQ@VeZak2Jy}^;yNfkb#KSn~L!2cRwNG6#{{}Q6fg{%k8A#dv=`}d@r
zh!v^p!_DU95kX$=s~9h?S6-rF&@T_@fi3=n0%CgIyv0SvD^MRJ$I~y*sTXkP(q2rq
z=*iGQ-EMuSOWvs;z}G?|n+2Bbs3+ZF%fD+lHT~_Bn=p;Y;3>X;+v|{Nov^%iYWMD-
zEAJ`eH2-@#a|7`TcF-CI9m`FbnfnFE-;4h`zasoPN__GJmG3Ez4_}X4IUuy&qDnLG
zrS^IW#)QeDiMn|iyZvZ=D(4tomA%Qi!g2+J(>e_bJ9guPO3Lk+DEuHY<U~Nclu`QC
z8BORa-`f>iCrCG=BxKicd~x&qT-dpO7TE(6^v9^jwnZ!K?VVu3#=D(P@M(;!-2(rh
zX+!-yT<~sVceb~ndin%yy5xkI&Q%Yw_%yLn>xQqi@Ko~n5#jC|lgoD~b@QF7<xh4i
zxg#`P#8+y_@Q=7V4$PSt^HjY;QX5w{fqY8NRq32BbKldDB7T7UhugQ@FO1+t{(XOg
zfMf&@ZT$1d`oG-yU-y3)U{{v^JAuCs)BX$b`CbU#H2*SM`zzw#2Mqs;_zm`_LH=LH
z4S%Kc>(tqwl%Bxt^<QStentLulI2h2ZRDTG|1sC{D}`Ts`F~QNLI0h?zdQTCqW;>q
z{1ep`T*LT-^Z!>D^H+kuwgLYn=nfv!`8&Zsn}WYm`L&kvClwEppHzOWto%yg@7ds=
z=m5YC6#(#SZul$q-(&7yvBzltg8i=u{44seJKLWGiWq)!|NpbkDa*rvZ4dxJ2LDlk
L&A5{3`RV@vcr%pj

literal 0
HcmV?d00001

diff --git a/tests/test_masterfile.py b/tests/test_masterfile.py
index 32c38d9c..31cb428c 100644
--- a/tests/test_masterfile.py
+++ b/tests/test_masterfile.py
@@ -16,3 +16,27 @@ def test_masterfile_read(data_path):
     assert nested_data["teachers"].shape == (11, 3)
     assert nested_data["schools"].shape == (10, 4)
     assert joined_data.shape == (40, 69)  # should be the same number of students as in the pupils df
+
+
+def test_masterfile_warns_duplicate_school(data_path, loguru_caplog):
+    """
+    Given a masterfile with two pupils in the same school, but the region is different for each pupil
+    When the masterfile is parsed
+    The nested data should have two schools, and there should be a loguru message about the duplicate ID
+    """
+    file_path = data_path / "masterfile_with_school_in_two_regions.xlsx"
+    nested_data = parse_masterfile(file_path)
+    # two schools, even though same id
+    assert nested_data["schools"].shape[0] == 2
+    assert "The following School IDs had duplicate information" in loguru_caplog.text
+
+
+def test_masterfile_no_duplicate_school(data_path, caplog):
+    """
+    Given a masterfile with no duplicate school details
+    When the masterfile is parsed
+    There should be no logging about duplicate school IDs
+    """
+    file_path = data_path / "example_masterfile.xlsx"
+    parse_masterfile(file_path)
+    assert "The following School IDs had duplicate information" not in caplog.text

From 28ae423a7b8fdabc475b0b03ac21e060a474f005 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Thu, 7 Sep 2023 18:13:48 +0100
Subject: [PATCH 7/8] Log warning when school has duplicate information

---
 src/rred_reports/reports/schools.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/rred_reports/reports/schools.py b/src/rred_reports/reports/schools.py
index c81fde9f..40871f1b 100644
--- a/src/rred_reports/reports/schools.py
+++ b/src/rred_reports/reports/schools.py
@@ -240,9 +240,9 @@ def populate_school_tables(school_df: pd.DataFrame, template_path: Path, report_
         columns, filter_function = column_and_filter
         filtered = filter_function(school_df, report_year)
         table_to_write = filtered[columns]
-        if index == 0 and (table_to_write.shape[0] != table_to_write.drop_duplicates().shape[0]):
+        if index == 0 and any(table_to_write.duplicated()):
             logger.warning(
-                "Table 1 has duplicate values that will be removed, suggests an issue with the masterfile school or teacher data:\n{school_data}",
+                "Duplicate students found, this suggests an issue with the masterfile school or teacher data. Table 1 data:\n{school_data}",
                 school_data=table_to_write.to_markdown(),
             )
         template_filler.populate_table(index + 1, table_to_write.drop_duplicates())

From f4c5ea186ca7ce74eb178c0ffd27545534cb5904 Mon Sep 17 00:00:00 2001
From: Stef Piatek <s.piatek@ucl.ac.uk>
Date: Thu, 7 Sep 2023 18:22:44 +0100
Subject: [PATCH 8/8] Add tabulate

To make CI happy, seems to be installed locally
---
 pyproject.toml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pyproject.toml b/pyproject.toml
index 98729596..72117441 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -36,6 +36,7 @@ dependencies = [
   "pypdf == 3.8.1",
   "python-docx == 0.8.11",
   "numpy == 1.24.2",
+  "tabulate == 0.9.0",
   "tomli == 2.0.1",
   "tomli-w == 1.0.0",
   "typer == 0.7.0",