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

Extend vehicle subclasses #453

Merged
merged 28 commits into from
Jun 25, 2020
Merged

Extend vehicle subclasses #453

merged 28 commits into from
Jun 25, 2020

Conversation

l-emele
Copy link
Contributor

@l-emele l-emele commented Jun 24, 2020

closes #425

Adds the following vehicles classes:

  • battery electric vehicle
  • fuel cell electric vehicle
  • grid supplied electric vehicle
  • plug-in hybrid electric vehicle

Adds the following energy converting devices and energy storage objects:

  • traction battery
  • motor
  • electric motor
  • traction motor
  • internal combustion engine

@l-emele l-emele requested review from akleinau and stap-m June 24, 2020 07:27
@l-emele l-emele marked this pull request as ready for review June 24, 2020 08:02
@l-emele
Copy link
Contributor Author

l-emele commented Jun 24, 2020

There are some merge conflicts, probably from #450. @akleinau : Can you please help with resolving these merge conflicts?

@akleinau
Copy link
Contributor

yes I'll fix, this is a bit weird

@l-emele
Copy link
Contributor Author

l-emele commented Jun 24, 2020

Thanks a lot.

@github-actions github-actions bot added the oeo-physical changes the oeo-physical module label Jun 24, 2020
@akleinau
Copy link
Contributor

ok @l-emele following problem at your side: when you commit sth, I guess your protege, changes the standard prefix of oeo-physical to another one which basically changes all class names in the file. Which creates merge conflicts.

@akleinau
Copy link
Contributor

akleinau commented Jun 24, 2020

see commit "add missing word in vehicle definition".
I changed it back and solved the conflicts though

@l-emele
Copy link
Contributor Author

l-emele commented Jun 24, 2020

Oh, that is not good. I've checked my protege preferences: They are as described in https://github.com/OpenEnergyPlatform/ontology/wiki/Numerical-Identifiers . Any other preferences apart from the "new entries" that I may have set wrong?

@akleinau
Copy link
Contributor

Oh, that is not good. I've checked my protege preferences: They are as described in https://github.com/OpenEnergyPlatform/ontology/wiki/Numerical-Identifiers . Any other preferences apart from the "new entries" that I may have set wrong?

actually I have no idea, @jannahastings?

@jannahastings
Copy link
Contributor

Typically this would be the New entities preferences, Entity IRI -> Start with -> Specified IRI (not 'active ontology IRI). If you think your settings should be correct, could you add a screenshot here?

@l-emele
Copy link
Contributor Author

l-emele commented Jun 24, 2020

grafik

@l-emele
Copy link
Contributor Author

l-emele commented Jun 24, 2020

Apart from my protege problems (and which @akleinau already fixed):

Are there any things to change here? As this PR adds a lot of classes I think it would be nice if can finalise the review and merge the PR before the OEO meeting tomorrow afternoon.

@jannahastings
Copy link
Contributor

my protege problems

Indeed your settings look correct :-(... I'm not really sure what else it might be. Let's monitor closely next time you are creating new classes, but I agree this PR is good to go

Copy link
Contributor

@stap-m stap-m left a comment

Choose a reason for hiding this comment

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

I am not done with the review but I just saw that there are many annotations with @de. I can fix it in the review but maybe you can also take a look at your settings here.

src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Outdated Show resolved Hide resolved
src/ontology/edits/oeo-physical.omn Show resolved Hide resolved
@l-emele
Copy link
Contributor Author

l-emele commented Jun 25, 2020

I am not done with the review but I just saw that there are many annotations with @de. I can fix it in the review but maybe you can also take a look at your settings here.

Thanks, I fixed my settings for that.

@l-emele l-emele requested a review from stap-m June 25, 2020 09:59
@stap-m stap-m merged commit ff57712 into dev Jun 25, 2020
@l-emele l-emele mentioned this pull request Jun 30, 2020
1 task
@christian-rli christian-rli deleted the feature/vehicles-#425 branch March 25, 2021 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
oeo-physical changes the oeo-physical module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vehicles need to be restructured
4 participants