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

New section library #132

Merged
merged 13 commits into from
Dec 31, 2021
Merged

New section library #132

merged 13 commits into from
Dec 31, 2021

Conversation

robbievanleeuwen
Copy link
Owner

As discussed in #75, a more orderly way of organising built-in geometries implemented in sectionproperties.

Consists of a new library module under the pre module, with built-in geometries organised into files based on their application. Currently consists of the following:

  • Standard sections
  • Steel sections
  • Concrete sections (consisting of 3 new built-in sections 🎉)
  • Bridge sections (to be added in future by @ccaprani)
  • Nastran sections

This also broke a fair number of examples in the docs which I have gone through and cleaned up.

#124 will also need to be slightly reworked to catch the new library structure.

@robbievanleeuwen robbievanleeuwen added the enhancement New feature or request label Dec 23, 2021
@connorferster
Copy link
Collaborator

connorferster commented Dec 31, 2021

Hi @robbievanleeuwen,

I made the changes in file naming: sections.py -> geometry.py -and- cross_section.py -> section.py. It seemed like it fit well within this branch/PR.

I see that the docs build so I think that means that I caught all of the updates that needed to occur in the doc string examples, too.

Shoot, I just realized that I will have to update all of the examples from @normanrichardson, too. Won't take too long once he is able to merge his PR.

Should norman's PR be merged before this one?

@robbievanleeuwen
Copy link
Owner Author

@connorferster yeah I think it makes sense to get #143 merged first then we can edit those files as needed in this PR.

@connorferster
Copy link
Collaborator

@robbievanleeuwen @Spectre5 I saw that the Lint checks were failing and so was sure to "blackify" all of the code in the directory. However, even after that, the linter is telling us that it would still modify files. I re-ran black on the whole section-properties directory and there are no modifications. What do you think might be going on here?

@Spectre5
Copy link
Contributor

Spectre5 commented Dec 31, 2021

I'd bet that you are using a different version locally than in the CI. This is one reason I like to use poetry to freeze the version and use poetry also in the CI files to make sure the same version is used in CI as locally, not just for black but for any other future code checkers or linters too (isort, pylint, flake8, mypy, etc). Removes a lot of these types of headaches, especially if more linting tools get added in the future. And makes sure that all developers are using the same versions locally too.

@robbievanleeuwen
Copy link
Owner Author

Yep I guess you have a different version of black locally as @Spectre5 suggested. I have the same output locally as the github action.

Also there appear to be a few issues with the docs build.

Another quick one with the sphinx-gallery - the mesh is not showing on any of the plots in the examples, e.g. see here. Don't have any experience with the gallery but maybe it's something to do with the image quality being saved?

@Spectre5
Copy link
Contributor

Spectre5 commented Dec 31, 2021

I'm guessing it is a resolution issue that @robbievanleeuwen has found. It you look closely, you can very faintly see an outline of the circle. Or a few edges in the second example output. One option would be to change resolution/dpi that sphinx-gallery uses to save the images (actually it normally just uses the figure size/dpi from the matplotlib figure itself). If that doesn't work, then another option could be to make the linewidth argument for plot_mesh be a keyword argument and then use something thicker, like lw=1.0 in the examples.

@Spectre5
Copy link
Contributor

Yet another option would be to allow passing figsize to plt.subplots and then using a larger figure. It probably should be larger anyway, so you could make the default larger (or else just the examples).

@connorferster
Copy link
Collaborator

It may also be caused by the alpha settings from the plot. Will check it out.

@connorferster connorferster merged commit 624e9f9 into v2 Dec 31, 2021
@connorferster connorferster deleted the new_section_library branch December 31, 2021 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants