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

[RTM] Bug and BoundsList #703

Merged
merged 3 commits into from
Jul 6, 2023
Merged

[RTM] Bug and BoundsList #703

merged 3 commits into from
Jul 6, 2023

Conversation

20chupin
Copy link
Contributor

@20chupin 20chupin commented Jun 28, 2023

Corrects a bug when running getting_started\pendulum.py and check the bound is not defined twice or with the same name


This change is Reviewable

@20chupin 20chupin changed the title Bug and BoundsList [WIP] Bug and BoundsList Jun 28, 2023
@20chupin 20chupin force-pushed the init_lists branch 3 times, most recently from 68597aa to 28893ab Compare July 4, 2023 20:38
Copy link
Collaborator

@Ipuch Ipuch left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @20chupin)


bioptim/limits/path_conditions.py line 654 at r1 (raw file):

                f"bounds['{item}'] is ambiguous because there are multiple phases. "
                f"To access the boundaries in this case, you should write bounds[phase]['{item}'] instead of "
                f"bounds['{item}']. I you didn't mean to use multiple phases, you may have specified bounds['{item}'] "

If

Copy link
Contributor Author

@20chupin 20chupin left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Ipuch)


bioptim/limits/path_conditions.py line 654 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

If

Done.

@20chupin 20chupin changed the title [WIP] Bug and BoundsList [RTR] Bug and BoundsList Jul 5, 2023
@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (5d440e3) 80.95% compared to head (09501c1) 80.95%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #703   +/-   ##
=======================================
  Coverage   80.95%   80.95%           
=======================================
  Files         119      119           
  Lines       13571    13571           
=======================================
  Hits        10986    10986           
  Misses       2585     2585           
Impacted Files Coverage Δ
bioptim/misc/options.py 77.27% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Ipuch
Ipuch previously approved these changes Jul 5, 2023
Copy link
Collaborator

@Ipuch Ipuch left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @20chupin)

Copy link
Member

@pariterre pariterre left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @20chupin)


bioptim/limits/path_conditions.py line 651 at r2 (raw file):

        if isinstance(item, str) and len(self.options) > 1:
            raise TypeError(

This should appear in the parent using "self.type" to get the type of option, and this should also be in the setter

@pariterre pariterre changed the title [RTR] Bug and BoundsList [RTM] Bug and BoundsList Jul 6, 2023
Copy link
Contributor Author

@20chupin 20chupin left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @Ipuch and @pariterre)


bioptim/limits/path_conditions.py line 651 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

This should appear in the parent using "self.type" to get the type of option, and this should also be in the setter

Done.

Copy link
Member

@pariterre pariterre left a comment

Choose a reason for hiding this comment

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

Tests?

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @20chupin)

Copy link
Contributor Author

@20chupin 20chupin left a comment

Choose a reason for hiding this comment

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

They passed !

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @20chupin)

@pariterre pariterre merged commit 851bc1f into pyomeca:master Jul 6, 2023
@20chupin 20chupin deleted the init_lists branch July 7, 2023 18:58
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.

3 participants