ORM issues in Django 4.2.5?

After upgrading to Django 4.2.5 recently, I experienced two ORM/Migration-related issues which I haven’t run across before. I was able to work around them, but wondered if others are seeing similar issues, and if so, if there is some way I can report the issues to the maintainers.

The first issue was that makemigrations didn’t see a new model I added. This model (Contract) has a ForeignKey field to an existing model (MarketCrop) with the related_name attribute set to ‘contracts’. I was able to force makemigrations to find it by temporarily adding
from .contract import Contract
to one of my other models.

After successfully migrating the new model, with existing market crops in the database, but before adding any contract records, I tried the following:

>>> from main.models import MarketCrop
>>> mc = MarketCrop.objects.all()[0]
>>> mc.contracts.all()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
AttributeError: 'MarketCrop' object has no attribute 'contracts'

Instead of the error, I expected to see

>>> mc.contracts.all()
<QuerySet []>

In order to work around this issue, I had to modify some methods in MarketCrop like this:

    def has_contracts(self):
        try:
            self.contracts
            return True
        except AttributeError:
            return False

    def get_basis_contracts(self):
        model_run_date = self.farm_year.get_model_run_date()
        if self.has_contracts():
            return self.contracts.filter(
                is_basis=True, contract_date__lte=model_run_date)
        else:
            return []

    def get_futures_contracts(self):
        model_run_date = self.farm_year.get_model_run_date()
        if self.has_contracts():
            return self.contracts.filter(
                is_basis=False, contract_date__lte=model_run_date)
        else:
            return []

This works well enough for my situation, since I just need those methods to return an iterable.
If I then create a contract referencing a market crop instance then do
market_crop.contracts.all()
I do get the expected (non-empty) QuerySet.

I’ve never come across either of these issues before.

Best,
Dow

We would likely need to see the models before and after the changes you’ve made to try and diagnose this. Part of creating an issue (if one is to be filed) is providing enough information to reproduce the problem.

Is this new model in the same app as MarketCrop, a different app that existed previously, or a new app just being created?

Thanks, Ken!
The new model ‘Contract’ is defined below. Note that it defined market_crop as a foreign key field with related_name=‘contracts’:

from datetime import datetime
from django.db import models
from .market_crop import MarketCrop

class Contract(models.Model):
    """
    Represents a futures or basis contract
    If a manual model_run_date is set, we should warn the user that contracts with
    contract date in the future are not displayed or included in budget or sensitivity.
    Otherwise, they might think they missed one and add a duplicate contract.
    """
    is_basis = models.BooleanField(
        default=False,
        help_text='Specifies a basis contract instead of a futures contract.')
    contract_date = models.DateField(
        default=datetime.today)
    bushels = models.IntegerField(default=0)
    price = models.FloatField(
        default=0, verbose_name="price per bushel")
    terminal = models.CharField(max_length=60, blank=True)
    contract_number = models.CharField(max_length=25, blank=True)
    delivery_start_date = models.DateField(
        null=True, blank=True)
    delivery_end_date = models.DateField(
        null=True, blank=True)
    market_crop = models.ForeignKey(
        MarketCrop, on_delete=models.CASCADE, related_name='contracts')
    class Meta:
        ordering = ['contract_date']

The pre-existing ‘MarketCrop’ model definition didn’t need to change at all. But because of the foreign key reference in Contract, it now has many contracts. It’s defined like this:

from django.core.validators import (
    MinValueValidator as MinVal, MaxValueValidator as MaxVal)
from django.db import models
from ext.models import FuturesPrice, MarketCropType
from .farm_year import FarmYear
from .fsa_crop import FsaCrop


class MarketCrop(models.Model):
    """
    A crop which can be marketed and which has a unique set of futures prices
    for a given county.
    """
    assumed_basis_for_new = models.FloatField(
        default=0, validators=[MinVal(-2), MaxVal(2)],
        help_text="Assumed basis for non-contracted bushels.")
    farm_year = models.ForeignKey(FarmYear, on_delete=models.CASCADE,
                                  related_name='market_crops')
    market_crop_type = models.ForeignKey(MarketCropType, on_delete=models.CASCADE)
    fsa_crop = models.ForeignKey(FsaCrop, on_delete=models.CASCADE,
                                 related_name='market_crops')
    price_factor = models.FloatField(
        default=1, validators=[
            MinVal(0),
            MaxVal(10, message="Ensure this value is less than or equal to 1000")],
        verbose_name='price sensititivity factor',
        help_text=('Percent of current futures price, reflected in detailed budget'))

    def __str__(self):
        return f'{self.market_crop_type}'

    def contracted_bu(self):
        return sum(c.bushels for c in self.get_futures_contracts())

    def basis_bu_locked(self):
        return sum(c.bushels for c in self.get_basis_contracts())

    def avg_contract_price(self):
        pairs = [(c.price*c.bushels, c.bushels) for c in self.get_futures_contracts()]
        if len(pairs) == 0:
            return 0
        else:
            amounts, bushels = zip(*pairs)
            tot_bu = sum(bushels)
            return sum(amounts) / tot_bu if tot_bu > 0 else 0

    def avg_locked_basis(self):
        pairs = [(c.price*c.bushels, c.bushels) for c in self.get_basis_contracts()]
        if len(pairs) == 0:
            return 0
        else:
            amounts, bushels = zip(*pairs)
            tot_bu = sum(bushels)
            return sum(amounts) / tot_bu if tot_bu > 0 else 0

    def has_contracts(self):
        try:
            self.contracts
            return True
        except AttributeError:
            return False

    def get_basis_contracts(self):
        model_run_date = self.farm_year.get_model_run_date()
        if self.has_contracts():
            return self.contracts.filter(
                is_basis=True, contract_date__lte=model_run_date)
        else:
            return []

    def get_futures_contracts(self):
        model_run_date = self.farm_year.get_model_run_date()
        if self.has_contracts():
            return self.contracts.filter(
                is_basis=False, contract_date__lte=model_run_date)
        else:
            return []

    def harvest_price(self):
        return self.harvest_futures_price_info(price_only=True)

    def sens_harvest_price(self, pf=None):
        if pf is None:
            pf = self.price_factor
        return self.harvest_futures_price_info(price_only=True) * pf

    def harvest_futures_price_info(self, priced_on=None, price_only=False):
        """
        Get the harvest price for the given date from the correct exchange for the
        crop type and county.  Note: insurancedates gives the exchange and ticker.
        """
        # TODO: consider returning a boolean "price locked" if the model run date
        # is after the expiration date of the post_ticker contract
        if priced_on is None:
            priced_on = self.farm_year.get_model_run_date()

        sql = """
            SELECT fp.id, fp.exchange, fp.futures_month, fp.ticker,
            fp.priced_on, fp.price
            FROM ext_futuresprice fp
            WHERE ticker=
                (select ticker from ext_tickers_for_crop_location
                 where crop_year=%s and state_id=%s and county_code=%s
                 and market_crop_type_id=%s
                 and (%s <= contract_end_date
                     or contract_end_date = last_contract_end_date)
                 order by contract_end_date limit 1)
            AND fp.priced_on <= %s
            ORDER BY priced_on desc limit 1
            """

        rec = FuturesPrice.objects.raw(
            sql, params=[self.farm_year.crop_year, self.farm_year.state_id,
                         self.farm_year.county_code, self.market_crop_type_id,
                         priced_on, priced_on])[0]
        return rec.price if price_only else rec

    def planted_acres(self):
        return sum((fc.planted_acres for fc in self.farm_crops.all()))

    def sens_farm_expected_yield(self):
        """ used in revenue buildup """
        return sum((fc.sens_farm_expected_yield() for fc in self.farm_crops.all()
                    if fc.has_budget()))

    def yield_factor(self):
        return sum((fc.yield_factor * fc.planted_acres
                    for fc in self.farm_crops.all())) / self.planted_acres()

    def county_bean_yield(self, yf=1):
        """ for indemnity calculations """
        ac = self.planted_acres()
        return (0 if self.market_crop_type_id != 2 or ac == 0 else
                sum((fc.sens_cty_expected_yield(yf) * fc.planted_acres
                     for fc in self.farm_crops.all())) / ac)

    def expected_total_bushels(self, yf=None):
        return sum((fc.sens_farm_expected_yield(yf=yf) * fc.planted_acres
                    for fc in self.farm_crops.all() if fc.has_budget()))

    def futures_pct_of_expected(self, yf=None):
        tot = self.expected_total_bushels(yf=yf)
        return (0 if tot == 0 else self.contracted_bu() / tot)

    def basis_pct_of_expected(self, yf=None):
        tot = self.expected_total_bushels(yf=yf)
        return (0 if tot == 0 else self.basis_bu_locked() / tot)

    def production_frac_for_farm_crop(self, farmcrop, yf=None):
        expbu = self.expected_total_bushels(yf)
        return (0 if expbu == 0 else
                (farmcrop.sens_farm_expected_yield(yf) * farmcrop.planted_acres /
                 expbu))

    class Meta:
        ordering = ['market_crop_type_id']

Both models are in the same app.

What files are each of these models in?

If these models are in the same app, they should both be in the same models.py file.

makemigrations didn’t see a new model I added

was able to force makemigrations to find it by temporarily adding from .contract import Contract

If your new model is in its own module, then yes you need to import it into your apps models module (either models.py or a models/__init__.py) so that Django can know about it. :slight_smile:

Ref: Models | Django documentation | Django

Thanks @shangxiao! – That explains the first issue I mentioned. Do you think it could also be causing the second issue (AttributeError) instead of empty QuerySet?

It seems to me this might be an item that could be improved in Django, since, as in my case, it often makes sense to have a model that doesn’t need to be imported by any other model, but which is integral to the application by virtue of its foreign key creating a one-to-many relation. In other words, I think Django migrations should be able to “know about it.”

Ideally the behavior of the ORM and Migrations should not depend on whether the models are in individual modules or all in the same module. As far as I know, the documentation doesn’t mention any discrepancy like this.

Thanks, Ken!

I have a models directory with an individual module/file for each model as described here. I didn’t see this restriction in the documentation, but I can move them into the same module as a work-around. It’s clear that would solve the first issue (with the migration) and maybe it will also solve the second issue (Attribute error instead of QuerySet([]) returned).

Do you think it could also be causing the second issue (AttributeError) instead of empty QuerySet?

Yep because the relationship is only known once the model is registered.

It seems to me this might be an item that could be improved in Django

I suppose Django could try to “discover” all models in all Python files from a starting directory similar to the way pytest discovers tests but that would add non-trivial complexity to the codebase… most folks are ok with models being the basis for which models are registered.

As far as I know, the documentation doesn’t mention any discrepancy like this.

The link I pasted earlier mentions this :slight_smile: Having said that if you feel that the documentation could be improved in any way, PRs are always welcome. The docs explain how to contribute but informally we follow the Diataxis framework: https://diataxis.fr/

After putting the two models in the same module, the AttributeError issue also appears to be solved. Thanks @KenWhitesell and @shangxiao!