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 overwriting part names #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

QuadratClown
Copy link

@QuadratClown QuadratClown commented Mar 5, 2024

Summary

This PR adds functionality to overwrite part names associated to supplier/manufacturer parts.

Problem

In some cases, parts can have multiple different manufacturers and MPNs, but are essentially the same. For example, there is a multitude of 10-pin 2.54mm 2x5 shrouded headers, which for almost all intents and purposes are identical.

For items like these, it does not make sense to keep seperate stock or even treat them differently in any way. However, currently inventree-part-import does not support such a scenario.

Description

This PR adds functionality to overwrite part names associated to supplier/manufacturer parts. With this, it's possible to store multiple similar supplier/manufacturer into the same part.

For example,

inventree-part-import --overwrite-part-name Header-254-16P-S 302-S161 BHR-16-VUA SBH11-PBPC-D08-ST-BK

will store multiple of the aforementioned headers into the same part, Header-254-16P-S. This keeps the parts minimal, allows keeping one stock item for all different supply/manufacturer parts and in general simplifies handling.

@30350n
Copy link
Owner

30350n commented Mar 5, 2024

For items like these, it does not make sense to keep seperate stock or even treat them differently in any way.

In general it does. If you ever run into issues with one of these part variations, it's very helpful if you are able to trace back where you got them from.

There's 2 proper solutions here:

  1. Use inventree_part_import as is and use the "Variant Of" setting to link multiple parts to a meta part.
    image
  2. Create multiple manufacturer parts for the same part. (inventree_part_import was designed in a way where one part always corresponds to exactly one manufacturer part, so changing this would require reworking it quite a lot).

@30350n
Copy link
Owner

30350n commented Mar 5, 2024

Also thanks for the PR and trying to resolve this, but there are two issues with this approach.

  1. As you already noticed "If multiple search terms are given, all resulting manufacturer/supplier parts will point to the same part.". This is pretty poor behaviour, as the default behavior is to treat each part separately from each other.
  2. From what I can tell the new ApiPart.name attribute doesn't serve any purpose, as it's always equal to the MPN. If anything, the whole option should probably just be renamed to override_mpn as that's essentially what it does, from what I can tell.

@30350n 30350n self-assigned this Mar 5, 2024
@QuadratClown
Copy link
Author

QuadratClown commented Mar 5, 2024

Use inventree_part_import as is and use the "Variant Of" setting to link multiple parts to a meta part.

Part variations require to keep separate stock, which is good idea in many cases, but not desired in every case

Create multiple manufacturer parts for the same part. (inventree_part_import was designed in a way where one part always corresponds to exactly one manufacturer part, so changing this would require reworking it quite a lot).

I'm not sure if I understand you correctly here, but in my understanding this is exactly what the PR does. It creates one part, with a user-assigned name, and creates supplier- and manufacturer-parts (with different search-terms) for that part

As you already noticed "If multiple search terms are given, all resulting manufacturer/supplier parts will point to the same part.". This is pretty poor behaviour, as the default behavior is to treat each part separately from each other

Yes, but the goal of this PR is to allow overriding this behaviour, to provide the aforementioned functionality as part of the script

From what I can tell the new ApiPart.name attribute doesn't serve any purpose, as it's always equal to the MPN. If anything, the whole option should probably just be renamed to override_mpn as that's essentially what it does, from what I can tell.

No, it serves a different purpose. Currently, mpn is used synonymously for both supplier/manufacturer part identifer AND regular part identifier. The function get_part_data returned a name field, but it was always just mpn. With the new data structure, it allows api_parts to have different name (part identifier) and mpn (manufacturer part/supplier part identifier)

@QuadratClown
Copy link
Author

I agree though that the naming scheme might not be ideal here. Maybe --use-common-part or similar would be more clear

@30350n
Copy link
Owner

30350n commented Mar 5, 2024

I'm not sure if I understand you correctly here, but in my understanding this is exactly what the PR does. It creates one part, with a user-assigned name, and creates supplier- and manufacturer-parts (with different search-terms) for that part.

Maybe. This might work for newly created parts. But it won't for already existing ones, because I search for existing Supplier/Manufacturer parts first and if they exists, just use them (and the part linked to them). So this could easily result in name clashes (because we'd be trying to rename another part to an existing name then).

No, it serves a different purpose. Currently, mpn is used synonymously for both supplier/manufacturer part identifer AND regular part identifier. The function get_part_data returned a name field, but it was always just mpn. With the new data structure, it allows api_parts to have different name (part identifier) and mpn (manufacturer part/supplier part identifier)

Still, the whole override could (and should) be done only in the PartImporter class. The suppliers module doesn't have to be touched.

@30350n
Copy link
Owner

30350n commented Mar 5, 2024

I think I had a similar situation in the past actually, where two different suppliers used slightly different MPNs for the same part. So I ended up with two parts, manufacturer parts and supplier parts each. The way I resolved this was by manually linking all the supplier parts to the same manufacturer part. And then deleting the duplicate manufacturer part and part. (I did this manually via the InvenTree UI).

Maybe an approach like this would be better here.

Basically a separate mode to --merge-manufacturer-parts (or for your case --merge-parts).
So you could import all your 2.54mm headers normally and then use a single inventree-part-import --merge-parts <MPN> <MPN> <MPN> ... call to combine them. The only problem with this approach is that one of my rules with this has been: No deleting.

@QuadratClown
Copy link
Author

Still, the whole override could (and should) be done only in the PartImporter class. The suppliers module doesn't have to be touched.

Ah, true. Agreed. I think I didn't do it that way because of the nested structure of the calls that create the necessary parts, as api_part is already passed through to all functions.

Basically a separate mode to --merge-manufacturer-parts (or for your case --merge-parts).

I like that approach a lot. That would also allow doing this in hindsight, which is better from a usability perspective.

The only problem with this approach is that one of my rules with this has been: No deleting.

A sensible approach here could be to always confirm deletions, e.g. by asking "this would delete parts xyz ... are you sure?"

@30350n
Copy link
Owner

30350n commented Mar 7, 2024

I like that approach a lot. That would also allow doing this in hindsight, which is better from a usability perspective.

Cool, then let's go with that! I'll try to find some time to throw together an implementation the next few days.

A sensible approach here could be to always confirm deletions, e.g. by asking "this would delete parts xyz ... are you sure?"

Yeah, I think I'll go for only marking the parts as "inactive" + a config option to enable automatic deleting + an "are you sure?" prompt, listing everything that's going to be deleted. Better be safe than sorry.

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.

2 participants