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

Enum with a different names #70

Closed
tfranzel opened this issue May 26, 2020 · 12 comments
Closed

Enum with a different names #70

tfranzel opened this issue May 26, 2020 · 12 comments
Assignees

Comments

@tfranzel
Copy link
Owner

Actually, one more note about the EvidenceRelationship schema. I have further updated the serializer to return two different relationships in a response:

    @extend_schema_field(serializers.ChoiceField(choices=EvidenceRelationship.choices))
    def get_expert_consensus_relationship(self, obj: Evidence) -> EvidenceRelationship:
        return self.get_consensus(obj, expert=True)

    @extend_schema_field(serializers.ChoiceField(choices=EvidenceRelationship.choices))
    def get_community_consensus_relationship(self, obj: Evidence) -> EvidenceRelationship:
        return self.get_consensus(obj, expert=False)

This does work, but it's generating two separate enum types in the spec:

    ExpertConsensusRelationshipEnum:
      enum:
      - PROVES
      - SUPPORTS
      - UNRELATED
      - INCONCLUSIVE
      - DISPUTES
      - DISPROVES
      - SPLIT
      type: string
    CommunityConsensusRelationshipEnum:
      enum:
      - PROVES
      - SUPPORTS
      - UNRELATED
      - INCONCLUSIVE
      - DISPUTES
      - DISPROVES
      - SPLIT
      type: string

Is there any way to tell drf-spectacular that these are the same enum?

Originally posted by @KunstDerFuge in #68 (comment)

@tfranzel
Copy link
Owner Author

ahh good point! the enum processing is not that smart. problem is that you use 2 different names, which cannot be avoided because you have it twice in your serializer.

when there are multiple names drf-spectacular would have to choose one, which is not obvious from a code point of view.

we use it mainly for stuff like country... severeal serializers have a country field and always the same choices so that gets replaced automatically because same name and same choices.

i think this would be a valuable feature addition.

@tfranzel
Copy link
Owner Author

partially related to #63

tfranzel added a commit that referenced this issue May 29, 2020
tfranzel added a commit that referenced this issue May 29, 2020
@tfranzel
Copy link
Owner Author

@KunstDerFuge i created a mechanism for solving your issue. you can consolidate the enums under one name with the new global setting:

https://drf-spectacular.readthedocs.io/en/latest/faq.html#i-get-warnings-regarding-my-enum-or-my-enum-names-have-a-weird-suffix

the new algorithm will also emit a warning for this issue. you still need the decorator to indicate that there is a enum there.

@KunstDerFuge
Copy link

I really appreciate this! I have to admit some ignorance here though; I'm not sure if I'll be able to test this. I'm not very skilled in integrating packages directly from GitHub rather than PyPI. I might be able to do it with a little guidance, though.

I also have to admit being pretty new to Django. Does this 'ENUM_NAME_OVERRIDES' flag go into the project-level settings.py?

So in my case, something like this would work?

'ENUM_NAME_OVERRIDES': {
    'ExpertConsensusRelationshipEnum': EvidenceRelationship,
    'CommunityConsensusRelationshipEnum': EvidenceRelationship
}

Sorry for asking so many questions! I appreciate all of the generous help you've already given.

@tfranzel
Copy link
Owner Author

pip install -U pip
pip uninstall drf-spectacular
pip install git+https://github.com/tfranzel/drf-spectacular.git

its good that you test this on master before i do a release. i usually try to collect a few fixes and improvements before doing a release.

thats right. that setting goes into your settings.py. it is namespaced like many apps.

REST_FRAMEWORK = {
 ....
}
SPECTACULAR_SETTINGS = {
    'ENUM_NAME_OVERRIDES': {
        'YourNameEnum': EvidenceRelationship.choices,
    }
}

your setting is not quite right. theoretically it would produce what the generator would do in its own. you want to set ONE name for all occurences of EvidenceRelationship.choices (like above).

congrats for being new to this and already knee deep into the tricky stuff 😆

p.s: you should also set some meta data for your API like title, version and description when you're at it https://drf-spectacular.readthedocs.io/en/latest/settings.html

@KunstDerFuge
Copy link

I see, thanks for the help. I've got the latest version now installed from master, but I'm not able to get the ENUM_NAME_OVERRIDES setting working. Importing my model class into settings.py is causing circular import issues. I tried referencing the name as a string instead:

SPECTACULAR_SETTINGS = {
    'ENUM_NAME_OVERRIDES': {
        'EvidenceRelationship': 'EvidenceRelationship.choices'
    }
}

(Which is probably incorrect) -- This gives the error:
ValueError: dictionary update sequence element #0 has length 1; 2 is required

My EvidenceRelationship enum is defined in models.py, and even if I remove every settings reference within models.py, I'm getting circular import issues (the unrelated "SECRET_KEY" setting must not be empty)

@KunstDerFuge
Copy link

If I do something a little bizarre, and put EvidenceRelationship into its own file, I'm able to successfully import it into both models and settings files, and then, like magic, it works! No warnings, just intelligent detection and singular reference to the EvidenceRelationship enum exactly as intended.

I don't mind this workaround personally, but I thought it would be worth mentioning. In my case, I use settings.AUTH_USER_MODEL within models.py, so I'm not able to import models into the settings file directly.

@tfranzel
Copy link
Owner Author

good point! actually that is not so bizarre. quite a few circular import issues are resolved that way.

you had the right idea actually. i thought about the import string, but opted for getting the functionality right first. since importing settings in the models is not that uncommon, its a thing worth adding. i overlooked that because the test do not have that issue.

i'll try to add the string importing. unfortunately its a bit more complicated since the DRF native string importing does not work without modification in this case.

jayvdb added a commit to jayvdb/drf-spectacular that referenced this issue Jun 1, 2020
@tfranzel
Copy link
Owner Author

tfranzel commented Jun 3, 2020

@KunstDerFuge you can throw out the bizarre stuff again. i had to resolve a few Enum issues myself and the direct approach was indeed quite cumbersome. i added a string import mechanism. you need to give the full path api.models.EvidenceRelationship. the .choices is not required as it detects the django choice classes automatically.

'EvidenceRelationship': 'EvidenceRelationship

please try it out and close the issue if it works as expected.

@KunstDerFuge
Copy link

Thank you, @tfranzel! This is working brilliantly for me.

I really appreciate the help, please let me know if I can be of help to the project in the future.

@KunstDerFuge
Copy link

Although, I believe I'm not able to close this issue, as I'm technically not the author.

Per my test, it is working as intended. 👍

@tfranzel
Copy link
Owner Author

tfranzel commented Jun 3, 2020

Awesome! Thanks, will do

@tfranzel tfranzel closed this as completed Jun 3, 2020
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