Skip to content

Commit e5272a0

Browse files
Reapply "Improve study_database robustness and add comprehensive tests"
This reverts commit da33750.
1 parent 6af7b0b commit e5272a0

File tree

2 files changed

+119
-2
lines changed

2 files changed

+119
-2
lines changed

src/studies/study_database.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ function save_study_database(h5path::AbstractString, db::study_database; kw...)
321321
# If :gparent column doesn't exist, create it from item names
322322
parent_groups = String[]
323323
for (i, item) in pairs(db.items)
324-
if !isnothing(item.name)
324+
if !isnothing(item.name) && !isempty(item.name)
325325
push!(parent_groups, IMAS.norm_hdf5_path(item.name))
326326
else
327327
Lpad = length(string(length(db.items)))
@@ -344,7 +344,7 @@ function save_study_database(h5path::AbstractString, db::study_database; kw...)
344344
if ismissing(pg) || !isa(pg, AbstractString) || isempty(pg)
345345
# Generate auto name for empty/missing values
346346
item = db.items[i]
347-
if !isnothing(item.name)
347+
if !isnothing(item.name) && !isempty(item.name)
348348
push!(updated_parent_groups, IMAS.norm_hdf5_path(item.name))
349349
else
350350
auto_item_name = "/item" * lpad(i, Lpad, "0")

test/runtests_study_database.jl

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,4 +167,121 @@ using FUSE.DataFrames
167167
end
168168
# mktempdir do ... end ensures temporary dir cleanup automatically
169169
end
170+
171+
@testset "Edge cases - empty names and :gparent handling" begin
172+
# Test items with nothing/empty names
173+
dd_test = FUSE.IMAS.json2imas(joinpath(@__DIR__, "..", "sample", "CAT_eq_ods.json"))
174+
175+
# Create items with various name states
176+
item_with_name = FUSE.study_database_item(name="valid_name", dd=deepcopy(dd_test))
177+
item_nothing_name = FUSE.study_database_item(name=nothing, dd=deepcopy(dd_test))
178+
item_empty_name = FUSE.study_database_item(name="", dd=deepcopy(dd_test))
179+
180+
# Test 1: DataFrame without :gparent column (should auto-create)
181+
@testset "Missing :gparent column" begin
182+
df_no_gparent = DataFrame(
183+
case = [1, 2, 3],
184+
status = ["success", "fail", "success"]
185+
)
186+
items_test = [item_with_name, item_nothing_name, item_empty_name]
187+
db_no_gparent = FUSE.study_database(df_no_gparent, items_test)
188+
189+
mktempdir() do save_dir
190+
h5file = joinpath(save_dir, "test_no_gparent.h5")
191+
# This should auto-create :gparent column
192+
FUSE.save_study_database(h5file, db_no_gparent; timer=false)
193+
194+
# Verify :gparent was created
195+
@test hasproperty(db_no_gparent.df, :gparent)
196+
@test db_no_gparent.df.gparent[1] == "/valid_name"
197+
@test db_no_gparent.df.gparent[2] == "/item2" # auto-generated
198+
@test db_no_gparent.df.gparent[3] == "/item3" # auto-generated
199+
end
200+
end
201+
202+
# Test 2: DataFrame with invalid :gparent values
203+
@testset "Invalid :gparent values" begin
204+
df_bad_gparent = DataFrame(
205+
gparent = ["valid/path", missing, ""], # mixed valid/invalid
206+
case = [1, 2, 3]
207+
)
208+
items_test = [item_with_name, item_nothing_name, item_empty_name]
209+
db_bad_gparent = FUSE.study_database(df_bad_gparent, items_test)
210+
211+
mktempdir() do save_dir
212+
h5file = joinpath(save_dir, "test_bad_gparent.h5")
213+
FUSE.save_study_database(h5file, db_bad_gparent; timer=false)
214+
215+
# Check that values were processed correctly
216+
@test db_bad_gparent.df.gparent[1] == "/valid/path" # normalized
217+
@test db_bad_gparent.df.gparent[2] == "/item2" # auto-generated from nothing name
218+
@test db_bad_gparent.df.gparent[3] == "/item3" # auto-generated from empty name
219+
end
220+
end
221+
222+
# Test 3: Large number of items (test padding)
223+
@testset "Auto-name padding" begin
224+
# Create 15 items to test padding (should use 2 digits)
225+
items_many = [FUSE.study_database_item(name=nothing, dd=deepcopy(dd_test)) for _ in 1:15]
226+
df_many = DataFrame(case = 1:15)
227+
db_many = FUSE.study_database(df_many, items_many)
228+
229+
mktempdir() do save_dir
230+
h5file = joinpath(save_dir, "test_padding.h5")
231+
FUSE.save_study_database(h5file, db_many; timer=false)
232+
233+
# Check padding is correct (should be 2 digits for 15 items)
234+
@test db_many.df.gparent[1] == "/item01"
235+
@test db_many.df.gparent[9] == "/item09"
236+
@test db_many.df.gparent[10] == "/item10"
237+
@test db_many.df.gparent[15] == "/item15"
238+
end
239+
end
240+
241+
# Test 4: Mixed valid and invalid names
242+
@testset "Mixed name scenarios" begin
243+
item1 = FUSE.study_database_item(name="group/subgroup", dd=deepcopy(dd_test))
244+
item2 = FUSE.study_database_item(name=nothing, dd=deepcopy(dd_test))
245+
item3 = FUSE.study_database_item(name="another_group", dd=deepcopy(dd_test))
246+
247+
df_mixed = DataFrame(
248+
gparent = ["/existing", missing, "no_slash"],
249+
case = [1, 2, 3]
250+
)
251+
db_mixed = FUSE.study_database(df_mixed, [item1, item2, item3])
252+
253+
mktempdir() do save_dir
254+
h5file = joinpath(save_dir, "test_mixed.h5")
255+
FUSE.save_study_database(h5file, db_mixed; timer=false)
256+
257+
# First keeps existing valid value
258+
@test db_mixed.df.gparent[1] == "/existing"
259+
# Second uses auto-generated (since name is nothing)
260+
@test db_mixed.df.gparent[2] == "/item2"
261+
# Third normalizes the invalid value
262+
@test db_mixed.df.gparent[3] == "/no_slash"
263+
end
264+
end
265+
266+
# Test 5: Loading and name stripping
267+
@testset "Loading with name stripping" begin
268+
# Create a db with known names
269+
item1 = FUSE.study_database_item(name="test_load", dd=deepcopy(dd_test))
270+
df_load = DataFrame(gparent = ["/test_load"], case = [1])
271+
db_load = FUSE.study_database(df_load, [item1])
272+
273+
mktempdir() do save_dir
274+
h5file = joinpath(save_dir, "test_load.h5")
275+
FUSE.save_study_database(h5file, db_load; timer=false)
276+
277+
# Load back
278+
db_loaded = FUSE.load_study_database(h5file)
279+
280+
# Name should have leading "/" stripped when loaded
281+
@test db_loaded.items[1].name == "test_load"
282+
# But :gparent should keep the "/"
283+
@test db_loaded.df.gparent[1] == "/test_load"
284+
end
285+
end
286+
end
170287
end

0 commit comments

Comments
 (0)