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

Allow to define model field name for specific field #17

Open
piranna opened this issue Jun 24, 2024 · 5 comments
Open

Allow to define model field name for specific field #17

piranna opened this issue Jun 24, 2024 · 5 comments

Comments

@piranna
Copy link

piranna commented Jun 24, 2024

At this moment, all the subfields are stored in the Model with a common prefix, that's the name of the CompositeField. I would like for a specific subfield to be stored with a different prefix, or no prefix at all. Something like this:

class A(CompositeField):
    b = CharField()
    c = CharField(composite_field_prefix=None)

class D(Model):
    e = A()

f = D()
# f.e.b == f.e_b
# f.e.c == f.c

I can contribute with a PR with some guidance on where I can do this, I have seen there's a reference to the %s_ prefix in the code but not sure where/how to store/modify it for a single field.

@piranna
Copy link
Author

piranna commented Jun 24, 2024

Another (maybe better) option is to be able to fully define the Model field name instead of just remove the prefix, so there's more flexibility.

@bikeshedder
Copy link
Owner

For a single field there is currently no way to change/remove the prefix. Think of it like that:

  • The CompositeField defines sub fields which all have their names. You can override the db column name just like in any regular django model. e.g. c = CharField(db_column="something_else").
  • When using the composite field you get to choose which prefix to use. It does default to the name of the field but can be overridden e.g. by passing an empty string as prefix.

We could change the code that prepend the prefix to the db_column to use a format instead. e.g. c = CharField(db_column="{}_something_else") so you could even use a suffix instead e.g. c = CharField(db_column="c_{}")

I'd have to check if Django does pass the string unchanged or if it causes a validation error because of the included {} characters.

But then... why?

I'm confused why you need something like that in the first place?

I'm really interested in better understanding your use case.

@piranna
Copy link
Author

piranna commented Jun 25, 2024

  • You can override the db column name just like in any regular django model. e.g. c = CharField(db_column="something_else")

Interesting, I didn't know about that, I'll take a look on it :-)

We could change the code that prepend the prefix to the db_column to use a format instead. e.g. c = CharField(db_column="{}_something_else") so you could even use a suffix instead e.g. c = CharField(db_column="c_{}")

I was thinking on using a dict to keep track of the actual names instead of generate them all the time based on the prefix, but the db_column argument could do the trick, just need to check it.

I'm confused why you need something like that in the first place?

I'm really interested in better understanding your use case.

I have a Model that inhering from another base one with generic fields, but in the child Model, it makes more sense to have accesible one of the fields as part of one of the Composite Fields, instead of being a Field of the base Model, so I was thinking on having it inside the Composite Field as an alias.

@piranna
Copy link
Author

piranna commented Jun 26, 2024

SystemCheckError: System check identified some issues:

ERRORS:
trcos_rf.Detection: (models.E007) Field 'drone_position' has column name 'position' that is used by another field.
	HINT: Specify a 'db_column' for the field.
trcos_rf.HistoricalDetection: (models.E007) Field 'drone_position' has column name 'position' that is used by another field.
	HINT: Specify a 'db_column' for the field.

So the db_column trick didn't work :-( I have looking for a way to define a property that use the value of the Model but was not able to easily get the actual model instance from the CompositeField instance...

@bikeshedder
Copy link
Owner

So you basically have two Models:

  • Detection with a drone_position field
  • HistoricalDetection with a drone_position field

And HistoricalDetection inherits from Detection?

If that's indeed the case it's not a CompositeField issue but rather a normal Django thing. A model inheriting from another model must not define the same field twice. Hence the models.E007 error you're seeing.

If you do want to create a "composite field" with existing fields you don't need this package but can just copy the idea and define your own Proxy class. This is also explained in the "How does it work?" chapter of the documentation: https://django-composite-field.readthedocs.io/latest/usage.html#how-does-it-work

You could also try using the Proxy class from the package but just be warned that some of it's API is an implementation detail and could change any time. Though I don't expect a lot of changes to this package so using it should be safe for the foreseeable future.

There is currently no support for defining the fields outside the composite field and just adding the proxy skipping the creation and adding of the fields. If you need this feature PRs are always welcome.

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

No branches or pull requests

2 participants