Skip to content

Use NeTEx authority short name if name is not present - #5698

Merged
leonardehrenfried merged 6 commits into
opentripplanner:dev-2.xfrom
unipoltech-maas:feat/agency-short-name
Feb 28, 2024
Merged

leonardehrenfried merged 6 commits into
opentripplanner:dev-2.xfrom
unipoltech-maas:feat/agency-short-name

Conversation

@cambio-deca

@cambio-deca cambio-deca commented Feb 20, 2024 •

Copy link
Copy Markdown

Summary

Add short name in Agency. Italian Netex file use ShortName tag instead Name in Authority.
If tag name is not filled and short name is filled, last one will be used as an agency name.

Unit tests

Test new field shortName.

@cambio-deca
cambio-deca requested a review from a team as a code owner February 20, 2024 10:41
@cambio-deca cambio-deca changed the title Feat/agency short name Feat: use agency short name if authority name is not present Feb 20, 2024
@cambio-deca cambio-deca changed the title Feat: use agency short name if authority name is not present feat: use agency short name if authority name is not present Feb 20, 2024
@leonardehrenfried

Copy link
Copy Markdown
Member

Hi there - welcome to OTP!

It looks like you are adding support for Italian Netex files.

Since this will require some attention from the maintainers of the the Netex code, I believe you will save time by showing up the developer meetings: https://github.com/opentripplanner/OpenTripPlanner/blob/dev-2.x/CONTRIBUTING.md#developer-meetings

There we talk about pull requests and assign reviews. It's also a good place to ask questions.

Lastly, all developers are in the Gitter chat room.

@codecov

codecov Bot commented Feb 21, 2024 •

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.71%. Comparing base (5da2268) to head (caf12a8).
Report is 13 commits behind head on dev-2.x.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5698      +/-   ##
=============================================
+ Coverage      67.69%   67.71%   +0.01%     
- Complexity     16369    16377       +8     
=============================================
  Files           1890     1891       +1     
  Lines          71812    71881      +69     
  Branches        7407     7411       +4     
=============================================
+ Hits           48611    48672      +61     
- Misses         20693    20702       +9     
+ Partials        2508     2507       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@leonardehrenfried
leonardehrenfried self-requested a review February 27, 2024 09:59
@leonardehrenfried

Copy link
Copy Markdown
Member

We spoke about this in the developer meeting today and we don't believe that the short name should end up in the internal data model. Using it as a fallback should simply be handled in the mapper code and you wouldn't even need to change the builder for this.

Comment thread src/main/java/org/opentripplanner/transit/model/organization/Agency.java Outdated

@leonardehrenfried leonardehrenfried left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the mapping code to the mapper.

Comment thread src/main/java/org/opentripplanner/netex/mapping/AuthorityToAgencyMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/netex/mapping/AuthorityToAgencyMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/netex/mapping/AuthorityToAgencyMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/netex/mapping/AuthorityToAgencyMapper.java Outdated
Comment thread src/main/java/org/opentripplanner/netex/mapping/AuthorityToAgencyMapper.java Outdated
@leonardehrenfried leonardehrenfried added the +NeTEx This issue is related to the Netex model/import. label Feb 27, 2024
@leonardehrenfried leonardehrenfried changed the title feat: use agency short name if authority name is not present Use NeTEx agency short name if authority name is not present Feb 27, 2024
@leonardehrenfried leonardehrenfried changed the title Use NeTEx agency short name if authority name is not present Use NeTEx authority short name if name is not present Feb 27, 2024
@leonardehrenfried
leonardehrenfried merged commit 6db3c8d into opentripplanner:dev-2.x Feb 28, 2024
t2gran pushed a commit that referenced this pull request Feb 28, 2024
@t2gran t2gran added this to the 2.5 (next release) milestone Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

+NeTEx This issue is related to the Netex model/import.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants