Skip to content

fix(rnd-21448): Fixtures.to_df() returns empty values for nested vessel/corporate_entities fields#571

Open
hansenfan wants to merge 6 commits into
masterfrom
rnd-21448
Open

fix(rnd-21448): Fixtures.to_df() returns empty values for nested vessel/corporate_entities fields#571
hansenfan wants to merge 6 commits into
masterfrom
rnd-21448

Conversation

@hansenfan

Copy link
Copy Markdown

RELATED TICKETS

https://vortexa.atlassian.net/jira/software/c/projects/RND/boards/465?selectedIssue=RND-21448

! Do not post related PRs here, unless they are open source !

CHANGELOG

  • Add convert_fixture_to_flat_dict to group vessel.corporate_entities by layer name before flattening
  • Fix tonestonnes typo in model, DEFAULT_COLUMNS, and docstrings
  • Correct docstring column names to match actual API response

TESTS

  • Verified before/after with live API: vessel.corporate_entities.effective_controller.label and
    time_charterer.label now populate correctly in to_df(), along with other fields. Has the ability to fill 'vessel.corporate_entities.effective_controller.label' which by definition exists in the schema, and if it exists like ticket says, but across all the fixtures it doesn't exist

COMMENTS

  • The ticket mentions vessels.corporate_entities.charterer.label but the API returns vessel (singular) and never includes charterer in corporate_entities, only at high level
  • Also worried that tones -> tonnes may be a breaking change
  • Also ticket said vessels, but it's vessel in the api structure and is only a single object, not a list

Copilot AI review requested due to automatic review settings July 13, 2026 13:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

vortexasdk/api/fixture.py:26

  • Renaming the Fixture field from tones to tonnes is a breaking change for SDK consumers who may still access fixture.tones. Consider keeping a deprecated alias property so existing code keeps working while the correct field name is used going forward.
    id: ID
    vessel: Optional[VesselEntity] = None
    laycan_from: Optional[str] = None
    laycan_to: Optional[str] = None
    tonnes: Optional[int] = None
    fixing_timestamp: Optional[str] = None
    vtx_fulfilled: Optional[bool] = None
    origin: Optional[Entity] = None
    destination: Optional[Entity] = None
    product: Optional[Entity] = None
    charterer: Optional[Entity] = None

"""Convert nested `Fixture` object to flat dictionary, keeping *cols*."""
as_dict = _group_fixture_attributes_by_layer(fixture)

formatted = flatten_dictionary(as_dict)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Would appreciate some input on this. I think because changing to tonnes could be a breaking change it is doing this to make sure that people don't have to modify anything but idk if that's a good practice

Comment thread vortexasdk/api/entity_flattening.py Outdated
Comment thread vortexasdk/endpoints/fixtures_result.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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