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

Support using auto with Postgres ArrayFields #288

Conversation

colinmcdonald22
Copy link

@colinmcdonald22 colinmcdonald22 commented Jul 8, 2023

Description

Previously, any django.contrib.postgres.fields.ArrayFields used in models had to be typed manually (say, as list[MyEnum]). Now they're compatible with auto, and resolve to a List of the type of base_field.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Issues Fixed or Closed by This PR

Fix #200

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@colinmcdonald22 colinmcdonald22 marked this pull request as draft July 8, 2023 19:40
@colinmcdonald22
Copy link
Author

Currently it seems like there's two issues here:

  1. I'm always adding Postgres dependencies to dev installs. I'd imagine we'd want to make sure tests pass w/o Postgres deps installed (for if they're not at runtime) - is there any precedent for splitting dev dependencies like this? I've only seen Django/Python versions handled specially, not any other optional dependencies.
  2. Even with Postgres dependencies added (verified w/ Poetry output in GH Actions logs), this is still failing - even though it passes locally. I'll dig more into this soon.

@baseplate-admin
Copy link
Contributor

Shouldn't we use psycopg3 ?

Django by default now uses psycopg3

@bellini666
Copy link
Member

Shouldn't we use psycopg3 ?

Django by default now uses psycopg3

The dependency here is a "dev" one and mostly for tests, meaning that if we used psycopg3 we would not be able to test on django 4.1 or older

@colinmcdonald22 colinmcdonald22 deleted the postgres-array-fields branch September 4, 2023 00:42
@noamsto
Copy link
Contributor

noamsto commented Oct 29, 2023

Why was this PR was closed and not merged?
Is there missing todos here?

@patrick91
Copy link
Member

@bellini666 do you think we should get this in?

@bellini666
Copy link
Member

@bellini666 do you think we should get this in?

Yes I do! It makes sense

Not sure why @colinmcdonald22 closed this, but I can take over it to ensure it will get in

@noamsto
Copy link
Contributor

noamsto commented Nov 22, 2023

@bellini666
Maybe this PR should be re-opened so it won't be forgotten ?

@dperetti
Copy link

I need this!

@bellini666
Copy link
Member

I need this!

You shall have this #567

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.

Add built-in support for List[] django model properties
6 participants