Mathadon issue172 validate experiment setup #210
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…periment-setup see #172
…/Mathadon/BuildingsPy into Mathadon-issue172_validateExperimentSetup
|
Yes I will do. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@tsnouidui Can you please revise this pull request and coordinate with @Mathadon
I had a brief look at it, but it needs quite some work on what I believe is your code.
_separate_mos_filesnot only separates the files, but also checks for tolerance. Please refactor so that the function does one thing (as its name implies)._separate_mos_fileschecks for"tolerance=1" in (l.replace(" ", "")).lower()Why "1". This function seems to fail if the tolerance is 5E-8._validate_experiment_setup(from Thierry) is still called, it returns error messages, but these returned messages are not used anywhere. Hence, the errors detected in this function seem to be ignored._missing_experiment_stoptimewhich I believe Filip added, replicates some of the functionality of_validate_experiment_setup.My suggestion is to refactor
_validate_experiment_setup(as it contains more tests than Filip's code) and make sure it returns all errors so that the can be reported by the code that calls it. This function is also quite bloated, maybe it is easier to have it do one thing as opposed to changing the behavior depending on its argumentname.