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

Added count of the number of elements that have missing values by period (and station) to Inventory dialog #8391

Merged
merged 10 commits into from Jun 23, 2023

Conversation

Vitalis95
Copy link
Contributor

Fixes #8342
@rdstern @lloyddewit @lilyclements @N-thony , this PR fixes part d) of the issue; adding in a count of the number of elements that have missing values by period (and station)

rdstern
rdstern previously approved these changes Jun 20, 2023
Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 and @lilyclements that looks great. I am not sure cum is needed, once we have cum1, but I'd like to keep both for now while we use it.
The one change is that when we save the results into a data frame, I don't think we need them in the output window as well. I hope that is easy to arrange. I am also approving, because it will be great to add this feature.

@Vitalis95
Copy link
Contributor Author

@rdstern ,results are no more in the output window when we save it.

@rdstern
Copy link
Collaborator

rdstern commented Jun 20, 2023

@Vitalis95 except you have now gone too far in the other direction!
a) When the results are in a data frame I still want to be able to have the comment reported in the output window

image

Here is the output window reporting the filts sub-dialog, but not that I used the inventory.

b) When the results are not saved into a dat frame, then they should go into the output window!
The results above are actually after I ran again, without saving into a data frame.

Thanks

@Vitalis95
Copy link
Contributor Author

@rdstern , have a look at it. Thanks

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 I could accept that. Except it wasn't a tibble before. Now it is and so only gives 10 rows and suggests the command to get more. That is ok in the output window, but I would have liked more rows in the maximised window. If it remains the same, then I suggest you need to make n larger than 10?

image

And I tried the summary dialog, which shows the comment fine - see figure above? I wonder why I don't get it here?

@Vitalis95
Copy link
Contributor Author

@rdstern , I checked with Patrick on the comment, probably there some controls which are not translated. That one can be done on a separate pull request

rdstern
rdstern previously approved these changes Jun 21, 2023
Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 this is great. @lloyddewit I hope the code is acceptable so it can be merged soon?

Copy link
Contributor

@lloyddewit lloyddewit left a comment

Choose a reason for hiding this comment

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

@Vitalis95 Thanks, just one small question

instat/dlgInventoryPlot.vb Outdated Show resolved Hide resolved
Co-authored-by: lloyddewit <57253949+lloyddewit@users.noreply.github.com>
Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@lloyddewit this still looks ok to me. This is also an improvement that will be good for the next workshops.

@lloyddewit lloyddewit changed the title Improvements to Inventory dialog Added count of the number of elements that have missing values by period (and station) to Inventory dialog Jun 23, 2023
@lloyddewit lloyddewit merged commit f552aaa into IDEMSInternational:master Jun 23, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add enhancements to Climatic > Inventory dialog
3 participants