Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 15, 2025

Overview

The combineRDBESDataObjects function can now be used to combine data from different hierarchies (e.g., H1 and H5), but previously did so silently, leading to structurally and statistically invalid objects. This PR adds an explicit warning to inform users when they are combining objects from different hierarchies.

Changes

Functionality

  • Added hierarchy detection logic that checks DEhierarchy values from both input objects
  • Issues a clear warning when combining objects with different hierarchies
  • The warning is non-blocking, allowing the operation to proceed for valid use cases
  • Gracefully handles edge cases (NULL or empty DE tables)

Warning Message

When combining different hierarchies, users now see:

Warning: Combining RDBESDataObjects from different hierarchies (1 and 5). 
This creates a mixed hierarchy object that may be structurally and statistically 
invalid for some analyses.

Documentation

  • Added @details section explaining when warnings are issued
  • Documented valid use cases for mixed hierarchies (fisheries overviews, annual reports, broader estimations)

Tests

  • Test for warning when combining different hierarchies (H1 and H5)
  • Test for no warning when combining same hierarchies
  • Test for no warning when one object has no DE table

Design Rationale

Following feedback from @rix133, this implementation:

  • Uses existing code patterns (similar to print.RDBESDataObject warning for mixed hierarchies)
  • Maintains backward compatibility - no breaking changes or new parameters
  • Issues warning regardless of strict parameter to ensure users are always informed
  • Allows operation to proceed (warning vs error) since mixed hierarchies are useful for some analyses

Use Cases

This change is particularly useful for:

  • Fisheries overviews requiring data from multiple hierarchies
  • Annual reports combining different sampling designs
  • Countries performing broader estimations across hierarchies

Users can now combine objects from different hierarchies with clear understanding of the implications.

Closes #[issue_number]

Original prompt

This section details on the original issue you should resolve

<issue_title>combineRDBESDataObjects should be more explicit in the handling of multiple hierarchies</issue_title>
<issue_description>function combineRDBESDataObjects can presently be used to combine data from different hierarchies, leading to an invalid object (both structurally and statistically). Still, being able to do that is useful to some uses (fisheries overviews, annual reports, countries doing more "brute" estimations).

A couple of alternatives are proposed to clarify this to user:

  • add an internal check for multiple hierarchies that if TRUE joins but prints a warning message
  • add a brute.force argument with default FALSE, but that if TRUE allows the joining and prints a warning message

@davidcurrie2001 your thoughts welcome here</issue_description>

Comments on the Issue (you are @copilot in this section)

@rix133 I think maybe a warning message and/or using the` strict` argument like we have used in other functions would be best but shoud we leave the default behaviour as is with a warning

Fixes #242


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits October 15, 2025 14:49
Co-authored-by: rix133 <6965560+rix133@users.noreply.github.com>
Co-authored-by: rix133 <6965560+rix133@users.noreply.github.com>
Copilot AI changed the title [WIP] Update combineRDBESDataObjects for multiple hierarchy handling Add warning when combining RDBESDataObjects from different hierarchies Oct 15, 2025
Copilot AI requested a review from rix133 October 15, 2025 14:54
@rix133 rix133 marked this pull request as ready for review October 15, 2025 14:55
Copy link
Contributor

@rix133 rix133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be stop instead of warning if strict is TRUE ie it warns if not strict and stops if strict about the hieararchy mismatch

@rix133 rix133 merged commit e0211d6 into dev Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants