GIT: Updates master to v1.7.0

[Gui-FernandesBR]

Quick and easy approval

[codecov]

Codecov Report

Attention: Patch coverage is 76.74419% with 50 lines in your changes missing coverage. Please review.

Project coverage is 75.97%. Comparing base (a1709f9) to head (9761e74).
Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
rocketpy/simulation/monte_carlo.py	37.03%	17 Missing ⚠️
rocketpy/plots/environment_analysis_plots.py	42.85%	16 Missing ⚠️
rocketpy/mathutils/function.py	75.00%	2 Missing ⚠️
rocketpy/plots/plot_helpers.py	91.66%	2 Missing ⚠️
rocketpy/prints/monte_carlo_prints.py	50.00%	2 Missing ⚠️
rocketpy/motors/hybrid_motor.py	50.00%	1 Missing ⚠️
rocketpy/motors/liquid_motor.py	50.00%	1 Missing ⚠️
rocketpy/motors/solid_motor.py	50.00%	1 Missing ⚠️
rocketpy/motors/tank.py	50.00%	1 Missing ⚠️
rocketpy/plots/aero_surface_plots.py	93.33%	1 Missing ⚠️
... and 6 more

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #731   +/-   ##
=======================================
  Coverage   75.97%   75.97%           
=======================================
  Files          95       95           
  Lines       11027    11027           
=======================================
  Hits         8378     8378           
  Misses       2649     2649           

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

[Gui-FernandesBR]

@phmbressan I was reflecting on the filename addition and got to a question. I'd like to ask your opinion on that.

If we release this version as it is, in the future we may suffer when adding new arguments to methods that has the filename argument. What will happen is that the filename will no longer be the last argument if we add new arguments to these methods.

I wonder if this is the case where we would benefit from doing something like this:

def thios_method(self, arg1, arg2, *, filename=None):

That way we could (potentially) add new arguments before the filename, this enforcing the filename always to be the last argument present in a method.

[Gui-FernandesBR]

[phmbressan]

If we release this version as it is, in the future we may suffer when adding new arguments to methods that has the filename argument. What will happen is that the filename will no longer be the last argument if we add new arguments to these methods.

I wonder if this is the case where we would benefit from doing something like this:

def thios_method(self, arg1, arg2, *, filename=None):

That way we could (potentially) add new arguments before the filename, this enforcing the filename always to be the last argument present in a method.

Well noticed. This seems like a good example of a future situation that we might get stuck with a breaking change. The solution you provided is alright, although in this case I find more straightforward to put it in the **kwargs. In both cases, I believe using it positionally only would cause an error.

def this_method(self, arg1, arg2, **kwargs):

I believe we should change the current behavior to one of the two solutions. Do you have any preferences?

[Gui-FernandesBR]

@phmbressan just to be clear... we would be removing the filename argument to **kwargs ?

[phmbressan]

just to be clear... we would be removing the filename argument to **kwargs ?

That is one possibility. It makes more sense if we had many paramters, like matplotlib does here (scroll down to the kwargs section):
https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.plot.html

The way you solved it should also work, if you prefer. The difference between them is that kwargs would allow any names to be a "named parameter".

[Gui-FernandesBR]

just to be clear... we would be removing the filename argument to **kwargs ?

That is one possibility. It makes more sense if we had many parameters, like matplotlib does here (scroll down to the kwargs section): https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.plot.html

The way you solved it should also work, if you prefer. The difference between them is that kwargs would allow any names to be a "named parameter".

@phmbressan I reflected on both options, they both are pretty similar to be honest. The idea is to enforce that no one ever uses the filename as a positional argument.

The only advantage I see in using the * operator is for type hinting purposes, since it would allow us to specify the type of the inputs. I know we do not use type hints in this project yet, but I have a dream one day someone will convince @MateusStano of using that!

Aside that fact, I see no other reason for any preference here.
What do you think? Do you agree?

I believe we should make a new PR implementing the solution we agree on. I am more inclined to use the * now, but honestly I would easily accept PRs with the alternative solution as well.

[Gui-FernandesBR]

just to be clear... we would be removing the filename argument to **kwargs ?

That is one possibility. It makes more sense if we had many parameters, like matplotlib does here (scroll down to the kwargs section): https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.plot.html
The way you solved it should also work, if you prefer. The difference between them is that kwargs would allow any names to be a "named parameter".

@phmbressan I reflected on both options, they both are pretty similar to be honest. The idea is to enforce that no one ever uses the filename as a positional argument.

The only advantage I see in using the * operator is for type hinting purposes, since it would allow us to specify the type of the inputs. I know we do not use type hints in this project yet, but I have a dream one day someone will convince @MateusStano of using that!

Aside that fact, I see no other reason for any preference here. What do you think? Do you agree?

I believe we should make a new PR implementing the solution we agree on. I am more inclined to use the * now, but honestly I would easily accept PRs with the alternative solution as well.

@phmbressan we need to address this filename/kwargs situation. It is currently blocking us from our next release.
Are we really going to change the filename arguments? How are we going to do so, and who is assigned to address it?
I don't have time to work on this right now, sadly.

Ideally, the next release should happen still in November.