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

imoClass Regex maybe incorrect? #153

Open
cmsdroff opened this issue May 23, 2023 · 7 comments
Open

imoClass Regex maybe incorrect? #153

cmsdroff opened this issue May 23, 2023 · 7 comments

Comments

@cmsdroff
Copy link
Contributor

Hi,

For the imoClass noted here https://app.swaggerhub.com/domains/dcsaorg/DCSA_DOMAIN/3.1.0#/components/schemas/imoClass (apologies can't find in Github)

I'm not sure if the imoClass should enforce the .x part of the value, we very often see the IMO class being passed as value 9 (miscellaneous) in movements and DG Notes which fails the regex specified in the class below, whilst class 9 (and others) can be split to a .x value it is not always that common looking at the DG Notes, and also the placard's used to identify the hazard.

So I'm questioning if it should be allowed to use just 9 or will it be enforced to use 9.1 or 9.2

also if this should be an enum to identify only specific and valid classes as you cannot have 9.6 for example?

We have had to override and remove the pattern from the schema to start using the DCSA schemas in the project, and left an open ticket relating to this one, welcome your thoughts.

imoClass:
      type: string
      pattern: '^[1-9]\.[1-9][A-Z]?$'
      description: |
        The hazard class code of the referenced dangerous goods according to the specified regulation.
      example: '1.4S'
@raisoman
Copy link

raisoman commented May 24, 2023

Indeed - I think I recall having seen single-digit IMO classes.

@HenrikHL
Copy link
Contributor

Hi @cmsdroff ,
As always - thanks for your input. We are currently working on the DG specifications - and the regEx for imoClass was mentioned today. Let me check once more for possible values with our DG experts. You might be right that the regEx needs to be improved.
We are trying to go away from enum values because of backward compatibility problems between minor API releases - hence the regEx.
Moving away from enum values in the DCSA specs will occur over the next beta versions...

@cmsdroff
Copy link
Contributor Author

Thanks @raisoman and @HenrikHL and understood about the use of regex for minor versions, to support the case I've taken a example snapshot from the DG Note that is used when booking / sending SI to carriers see below, the 9 is singular, looking at the EDI files its also presented like this

DGS+IMD+9+3077++3+F-AS-F++++9:EHSM'

again after the IMD you see the value of 9

Screenshot 2023-05-25 at 14 42 42

Looked across a couple of large shippers DG notes and it's consistent between them currently, hope its useful.

@HenrikHL
Copy link
Contributor

HenrikHL commented May 25, 2023

Hi @cmsdroff,
DG experts have just reverted to me and you are correct. The following values should be accepted:
1.1A
1.1B, 1.2B, 1.4B
1.1C, 1.2C, 1.3C, 1.4C
1.1D, 1.2D, 1.4D, 1.5D
1.1E, 1.2E, 1.4E
1.1F, 1.2F, 1.3F, 1.4F
1.1G, 1.2G, 1.3G, 1.4G
1.2H, 1.3H
1.1J, 1.2J, 1.3J
1.2K, 1.3K
1.1L, 1.2L, 1.3L
1.6N
1.4S
2.1
2.2
2.3
3
4.1
4.2
4.3
5.1
5.2
6.1
6.2
7
8
9

I will probably revert from a regEx to an Enum as I am also told the following:
The IMO classes will probably not changed unless a significant rework of the worldwide DG regulations will take place…

@cmsdroff
Copy link
Contributor Author

great news thanks for the speedy update @HenrikHL :)

Will leave to you if you want to close or link to commit

@HenrikHL
Copy link
Contributor

Hi @cmsdroff
We have talked internally about this and have agreed to make the imoClass attribute a string with allowed values specified in the following dgimoclass.csv file
Let me know what you think

@HenrikHL
Copy link
Contributor

HenrikHL commented May 31, 2023

I did make a regular expression:
^((1\.((1[A-GJL])|(2[B-HJKL])|(3[CFGHJKL])|(4[B-GS])|(5D)|(6N)))|([24]\.[1-3])|([56]\.[12])|[3789])$
Tested here: https://regex101.com/r/NR5JxB/1
To validate input - but we agreed that a list on GitHub would be better in case allowed values change

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

3 participants