TOMToolkit / tom_base

The base Django project for a Target and Observation Manager
https://tom-toolkit.readthedocs.io
GNU General Public License v3.0
23 stars 42 forks source link

Target match management not applied if targets created directly by management command #970

Open rachel3834 opened 1 week ago

rachel3834 commented 1 week ago

Describe the bug I've implemented the TOM's custom match manager with the goal of making sure I don't ingest duplicate targets at the same RA, Dec, which may have different names stemming from different surveys.

Following the example in the documentation, I've been able to implement this OK, but based on the results of unittests, the match manager isn't being called when I use Django's standard functions for object creation in management commands. E.g.:

t = Target.objects.create(
            name=name,
            ra=ra,
            dec=dec,
            type='SIDEREAL',
            epoch=2000
        )

While the Target create() method saves the new Target to the database, it does not call the validate_unique() method on the (custom) Target object in the process.

This can be done by creating a new Target, then immediately validating it, but then I'd need logic to delete it if it proves to be a duplicate, which seems cumbersome.

As a temporary workaround, I've implemented a validator which performs the logic I need, linking this here for reference.

jchate6 commented 1 week ago

I have added a warning to the docs.

Do you think this will be sufficient?

rachel3834 commented 1 week ago

I think it is good, but it would be good to spell out the circumstances where this will happen for less experienced users. I suggest giving an example code snippet to draw attention, such as

target, created = Target.objects.get_or_create( ...)

On Thu, Jun 27, 2024 at 12:27 PM Joey Chatelain @.***> wrote:

I have added a warning https://github.com/TOMToolkit/tom_base/blob/ecb6e50fdc80e04eba8545dbc960cfa4fc789c63/docs/targets/target_matcher.rst?plain=1#L121 to the docs.

Do you think this will be sufficient?

— Reply to this email directly, view it on GitHub https://github.com/TOMToolkit/tom_base/issues/970#issuecomment-2195514259, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPJA3BJ4KVO7UO6JIF2JD3ZJRRQ3AVCNFSM6AAAAABKAQJJC2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJVGUYTIMRVHE . You are receiving this because you authored the thread.Message ID: @.***>

jchate6 commented 1 week ago

Next step here is to consider implementing custom create and get_or_create methods for the match manager. this would allow a user to use target.matches.create() to create a target in the same way that target.objects.create() does, except with validation built in.