Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TP2000 1650 commodities snapshot behaviour #1379

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Luisella21
Copy link
Contributor

@Luisella21 Luisella21 commented Jan 10, 2025

TP2000-1650 commodities snapshot behaviour with effective_only=True

Using CommodityCollectionLoader.load(effective_only=True) gives an error, because a filter is applied to a non existing field.
Added a small test showing the issue, and changed the code to fix it.

  • Requires migrations: No
  • Requires dependency updates: No

@Luisella21 Luisella21 requested a review from a team as a code owner January 10, 2025 11:55
@Luisella21 Luisella21 changed the title TP2000 1650 snapshot crash TP2000 1650 snapshot behaviour Jan 10, 2025
@Luisella21 Luisella21 changed the title TP2000 1650 snapshot behaviour TP2000 1650 commodities snapshot behaviour Jan 10, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

Might this file be better served as an addition to .gitignore to allow for local, personalised .envrc configurations?

Comment on lines +97 to +98
def test_CommodityCollectionLoader(seed_database_with_indented_goods):
# Test that effective_only does not crash the code
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps the comment could form part of the test name to reflect what in particular is being tested.

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